[Cases] Increase default page size cases table and save preferences in url/localStorage #144228

adcoelho · 2022-10-31T11:22:44Z

Summary

Issues:
#131806
#140008

Increase the default table size of the cases table to 10
Changed the available page sizes to 10, 25, 50 and 100
Save the visualization preferences of the cases table in localStorage
Display the current visualization preferences of the cases table in the URL
This logic is not applied if the cases table is opened in a modal

Screenshots

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios
This was checked for cross-browser compatibility

Fixes #140008

Release notes

Increase the default table size of the cases table to 10
Save the visualization preferences of the cases table in localStorage
Display the current visualization preferences of the cases table in the URL

peteharverson · 2022-11-01T17:51:55Z

x-pack/plugins/cases/public/common/hooks.ts

+  const [localStorageQueryParams, setLocalStorageQueryParams] =
+    useLocalStorage<LocalStorageQueryParams>('cases.list.preferences');
+
+  const setUrlQueryParams = useCallback(


I think we need to be careful about setting parameters in the URL when using the case list as a modal inside another page / app as this can have unwanted side effects. For example, when attaching ML embeddables to a case, the page is expecting job ID parameters to be present in the URL and the job selector flyout is incorrectly being opened when the URL is being manipulated.

It might make more sense to only persist page settings in the URL if you are on the Cases list page, and for the modal to always default to 10 cases per page for example.

I think we need to be careful about setting parameters in the URL when using the case list as a modal inside another page / app as this can have unwanted side effects.

The current behavior should remove the modal-specific URL parameters as soon as the modal is unmounted.

When I noticed that the alerts page had some URL parameters of its own, I made it so that those would remain on the page and page + perPage + sortField + sortOrder disappear when you close the modal.

Did it behave differently with these job ID params you mention?

I did find a somewhat related bug though.

If we filter by status in the all-cases view and pick Closed, the sortField becomes closedAt.

This works fine in this view but if then we navigate to Alerts and pick Add existing case to alert we shouldn't be able to filter by that status there so the table doesn't work properly. Existing Cases imply Open or In Progress only.

x-pack/plugins/cases/public/containers/constants.ts

peteharverson

Tested and overall this is looking like a great enhancement. Just wonder if we need to limit the scope so that it doesn't include the modal?

jonathan-buttner

Looking good! Left some preliminary feedback

x-pack/plugins/cases/public/common/hooks.ts

x-pack/plugins/cases/public/components/all_cases/all_cases_list.tsx

x-pack/plugins/cases/public/containers/constants.ts

x-pack/plugins/cases/public/common/utils.test.tsx

x-pack/plugins/cases/public/common/hooks.ts

elasticmachine · 2022-11-03T15:47:49Z

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

elasticmachine · 2022-11-03T15:47:50Z

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

x-pack/plugins/cases/public/components/all_cases/all_cases_list.tsx

x-pack/plugins/cases/common/ui/types.ts

x-pack/plugins/cases/public/common/hooks.test.tsx

x-pack/plugins/cases/public/components/all_cases/all_cases_list.tsx

x-pack/plugins/cases/public/components/all_cases/use_url_state.test.tsx

x-pack/plugins/cases/public/components/all_cases/use_url_state.tsx

x-pack/plugins/cases/public/components/all_cases/utils.ts

x-pack/plugins/cases/public/components/all_cases/use_url_state.test.tsx

EricDavisX · 2022-11-07T17:30:44Z

Heya @adcoelho I added the ci:cloud-deploy label, next time a commit is pushed up it'll start building it and I can easily take a quick peek at it from the feature/user side. I can share details of the process/usage of the cloud build with you async.

* Started saving the Cases list visualization preferences in localStorage and in the url

… url params. * Created unit tests for the useUrlState hook.

… modal. * Update tests.

…tests.

* Created app specific localStorage keys for all cases list preferences. * Started preserving existing url parameters when applicable. * Wrapped stringify call in try/catch block.

x-pack/plugins/cases/public/components/all_cases/all_cases_list.tsx

…ases list preferences.

…ge-size-cases-table

cnasikas

Tested without any issues! Code LGTM! 🚀

adcoelho · 2022-11-10T08:06:51Z

@elasticmachine merge upstream

…olution.

…ge-size-cases-table

machadoum

Hey!

We have implemented a set of utility hooks inside the security solution app that could have helped here, like useReplaceUrlParams and useGetInitialUrlParamValue.

Unrelated question: I noticed that the cases page has sourcerer and timerange query params. Are they used for anything? Does it make sense to delete it?

machadoum · 2022-11-10T13:36:39Z

x-pack/plugins/cases/public/components/all_cases/use_all_cases_query_params.tsx

+
+      if (!isEqual(newUrlParams, urlParams)) {
+        try {
+          history.push({


Every time the user updates one of the query param values this code is stacking a history entry. So if users want to go to back to the previous page they have to click many times the back button.

The usage of history.push makes the implementation inconsistent with how other query parameters work inside Security solution. We use history.replace to avoid stacking many entries to the history stack.

Cases are being used in Observability, Security solution, and the Stack. We decided it is better to push the history so the user can go back to the previous filtering. Why did you decide to not keep the history in your queries?

Hi @machadoum , thanks for the review!

I noticed that the cases page has sourcerer and timerange query params. Are they used for anything? Does it make sense to delete it?

These are actually not ours, they come from your side and I tried to leave everything as is. In fact, at some point, the logic did not take them into account and the security tests failed.

That decision preceded me. But I can imagine that one reason is that security has many filters, and it could take many clicks for a user to return to the previous page.
Especially if you consider that filters are added the first time a page loads, if a user opens the cases page and decides to go to the previous page immediately, it wouldn't work. They would have to click at least twice. I tested some pages, and it is very inconsistent across applications. For example, on the Observability Alerts page, the user must click four times the back button to leave the page.

My experience as a user of other applications is that the history is being kept but I am not sure what is best. @mdefazio Any input on this?

Unrelated question: I noticed that the cases page has sourcerer and timerange query params. Are they used for anything? Does it make sense to delete it?

@machadoum I think this params are used by the timeline, the TL bottom bar is enabled in Cases (in Security)

Since the back button behaviour is inconsistent across different pages and areas, let's not block this PR because of it. The only scenario that looks like a bug to me is the back button behaviour when the page loads.
Given the following scenario:

The user opens the menu

The user clicks on the cases link

Cases page loads

The user clicks the browser back button
Expected behaviour: It should navigate back to the previous page
Current behaviour: It stays on the cases page

As discussed offline, I will merge this PR now to make FF and created an issue for this bug behavior .

I will handle this issue during the bug-fixing phase sometime this week.

Thanks for the review @machadoum 🙌

I Agree, let's not block it.
But the workflow described by @machadoum is weird. I think we should discuss this issue, the back button behavior should be consistent across the app.
(IMHO, I don't think the expected behavior of the browser's back button is to roll back the filters.)
cc @YulNaumenko

Thank you both for the feedback. the back button behavior is inconsistent across Kibana. For example, Discovery & Lens keeps the filtering in the history.

adcoelho · 2022-11-14T11:20:09Z

@elasticmachine merge upstream

kibana-ci · 2022-11-14T11:56:24Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`cases`	508	511	+3

Async chunks

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

id	before	after	diff
`cases`	344.8KB	346.9KB	+2.1KB
`securitySolution`	9.6MB	9.6MB	+24.0B
total			+2.1KB

Page load bundle

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

id	before	after	diff
`cases`	126.6KB	126.6KB	+76.0B

Unknown metric groups

ESLint disabled in files

id	before	after	diff
`osquery`	1	2	+1

ESLint disabled line counts

id	before	after	diff
`enterpriseSearch`	19	21	+2
`fleet`	59	65	+6
`osquery`	108	113	+5
`securitySolution`	441	447	+6
total			+19

Total ESLint disabled count

id	before	after	diff
`enterpriseSearch`	20	22	+2
`fleet`	67	73	+6
`osquery`	109	115	+6
`securitySolution`	518	524	+6
total			+20

History

💚 Build #86950 succeeded 5ad3ee6
💚 Build #86898 succeeded 59c3848
💔 Build #86620 failed 5d70864
💔 Build #86588 failed 4283fe1
💔 Build #86394 failed 0c15670eea2a161e3c05cdb5b0709c03ac9a97de

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

adcoelho added release_note:enhancement Team:ResponseOps Platform ResponseOps team (formerly the Cases and Alerting teams) Feature:Cases Cases feature v8.6.0 labels Oct 31, 2022

adcoelho force-pushed the increase-default-page-size-cases-table branch 3 times, most recently from 0fc9d07 to ba4228b Compare October 31, 2022 16:01

peteharverson changed the title ~~Increase default page size cases table and save preferences in url/localStorage~~ [Cases] Increase default page size cases table and save preferences in url/localStorage Nov 1, 2022

peteharverson reviewed Nov 1, 2022

View reviewed changes

x-pack/plugins/cases/public/containers/constants.ts Show resolved Hide resolved

peteharverson reviewed Nov 1, 2022

View reviewed changes

adcoelho force-pushed the increase-default-page-size-cases-table branch from 6874ebb to 4e10288 Compare November 2, 2022 13:16

jonathan-buttner reviewed Nov 2, 2022

View reviewed changes

adcoelho force-pushed the increase-default-page-size-cases-table branch 4 times, most recently from 33e289c to 99855af Compare November 3, 2022 12:48

adcoelho added the ci:cloud-deploy Create or update a Cloud deployment label Nov 3, 2022

jonathan-buttner reviewed Nov 3, 2022

View reviewed changes

x-pack/plugins/cases/public/common/utils.test.tsx Outdated Show resolved Hide resolved

x-pack/plugins/cases/public/common/hooks.ts Outdated Show resolved Hide resolved

x-pack/plugins/cases/public/common/hooks.ts Outdated Show resolved Hide resolved

adcoelho force-pushed the increase-default-page-size-cases-table branch from 99855af to e0df6be Compare November 3, 2022 15:45

adcoelho marked this pull request as ready for review November 3, 2022 15:47

adcoelho requested a review from a team as a code owner November 3, 2022 15:47

adcoelho force-pushed the increase-default-page-size-cases-table branch from e0df6be to 4a72a7e Compare November 3, 2022 15:51

peteharverson reviewed Nov 3, 2022

View reviewed changes

x-pack/plugins/cases/public/components/all_cases/all_cases_list.tsx Outdated Show resolved Hide resolved

adcoelho removed the ci:cloud-deploy Create or update a Cloud deployment label Nov 3, 2022

adcoelho force-pushed the increase-default-page-size-cases-table branch from 4a72a7e to 5a82d16 Compare November 4, 2022 10:01

adcoelho requested a review from a team as a code owner November 4, 2022 10:01

adcoelho force-pushed the increase-default-page-size-cases-table branch from 5a82d16 to cf4c1dd Compare November 4, 2022 14:31

adcoelho force-pushed the increase-default-page-size-cases-table branch from cf4c1dd to 7c42e0d Compare November 7, 2022 08:49

cnasikas reviewed Nov 7, 2022

View reviewed changes

adcoelho force-pushed the increase-default-page-size-cases-table branch from 7c42e0d to dec7f74 Compare November 7, 2022 17:00

EricDavisX added the ci:cloud-deploy Create or update a Cloud deployment label Nov 7, 2022

adcoelho force-pushed the increase-default-page-size-cases-table branch from 63d0ae1 to 0c15670 Compare November 8, 2022 18:13

adcoelho added 7 commits November 9, 2022 12:51

* Increased the DEFAULT_TABLE_LIMIT for the cases list to 10

d88e84c

* Started saving the Cases list visualization preferences in localStorage and in the url

* Updated existing tests to include the MemoryRouter wrapper.

b207ac6

* Added unmount logic to the useUrlState hook to remove hook specific…

32cf8bf

… url params. * Created unit tests for the useUrlState hook.

* Save cases list preferences only if the table is not displayed in a…

c593936

… modal. * Update tests.

* Moved all_cases_list specific hooks to a new location. Updated the …

65bb39c

…tests.

* Addressing PR comments.

29e2117

* Removed localStorage mocks from hook tests.

4283fe1

* Created app specific localStorage keys for all cases list preferences. * Started preserving existing url parameters when applicable. * Wrapped stringify call in try/catch block.

adcoelho force-pushed the increase-default-page-size-cases-table branch from 0c15670 to 4283fe1 Compare November 9, 2022 11:51

cnasikas reviewed Nov 9, 2022

View reviewed changes

x-pack/plugins/cases/public/components/all_cases/all_cases_list.tsx Show resolved Hide resolved

adcoelho added 2 commits November 9, 2022 14:20

* Take into account the previous values when partially updating the c…

558ff02

…ases list preferences.

Merge remote-tracking branch 'upstream/main' into increase-default-pa…

5d70864

…ge-size-cases-table

cnasikas approved these changes Nov 9, 2022

View reviewed changes

kibanamachine and others added 3 commits November 10, 2022 03:06

Merge branch 'main' into increase-default-page-size-cases-table

59c3848

* Move the CasesContext between the Router and Homepage in security_s…

ae9f1a2

…olution.

Merge remote-tracking branch 'upstream/main' into increase-default-pa…

5ad3ee6

…ge-size-cases-table

machadoum self-requested a review November 10, 2022 13:25

machadoum reviewed Nov 10, 2022

View reviewed changes

Merge branch 'main' into increase-default-page-size-cases-table

8894a27

machadoum approved these changes Nov 14, 2022

View reviewed changes

adcoelho merged commit 46a7197 into elastic:main Nov 14, 2022

kibanamachine added the backport:skip This commit does not require backporting label Nov 14, 2022

[Cases] Increase default page size cases table and save preferences in url/localStorage #144228

[Cases] Increase default page size cases table and save preferences in url/localStorage #144228

Uh oh!

Conversation

adcoelho commented Oct 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Screenshots

Checklist

Release notes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

peteharverson left a comment

Choose a reason for hiding this comment

Uh oh!

jonathan-buttner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

elasticmachine commented Nov 3, 2022

Uh oh!

elasticmachine commented Nov 3, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

EricDavisX commented Nov 7, 2022

Uh oh!

Uh oh!

cnasikas left a comment

Choose a reason for hiding this comment

Uh oh!

adcoelho commented Nov 10, 2022

Uh oh!

machadoum left a comment

Choose a reason for hiding this comment

Uh oh!

machadoum Nov 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

machadoum Nov 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cnasikas Nov 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

machadoum Nov 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

adcoelho commented Oct 31, 2022 •

edited

Loading

machadoum Nov 10, 2022 •

edited

Loading

machadoum Nov 14, 2022 •

edited

Loading

cnasikas Nov 14, 2022 •

edited

Loading

machadoum Nov 14, 2022 •

edited

Loading

kibana-ci commented Nov 14, 2022 •

edited

Loading