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

[Cases] Refactor Cases List Filters #169371

Merged
merged 36 commits into from
Nov 8, 2023
Merged

[Cases] Refactor Cases List Filters #169371

merged 36 commits into from
Nov 8, 2023

Conversation

jcger
Copy link
Contributor

@jcger jcger commented Oct 19, 2023 •
edited
Loading

Meta #167651

Description

This PR refactors the filters to be multi select

Screenshot

QA

  • Filters work as multi select now
  • 'All' has been removed as option for status and severity. Check that old urls, local storage values do not break anything
  • Review the cases list in the modal when adding a timeline to a case

@jcger jcger added Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) release_note:skip Skip the PR/issue when compiling release notes Feature:Cases Cases feature v8.12.0 labels Oct 30, 2023
jcger
jcger commented Nov 6, 2023
View reviewed changes
x-pack/plugins/cases/public/components/all_cases/table_filters.test.tsx
expect(onFilterChanged).toBeCalledWith({ status: [CaseStatuses.closed] });
});

it('should remove tag from selected tags when tag no longer exists', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this tests removed here are tested in the multi select filter component now

@jcger jcger marked this pull request as ready for review November 6, 2023 13:12
@jcger jcger requested a review from a team as a code owner November 6, 2023 13:12
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

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

@jcger jcger requested a review from a team as a code owner November 6, 2023 20:09
adcoelho
adcoelho approved these changes Nov 7, 2023
View reviewed changes
Copy link
Contributor

@adcoelho adcoelho left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Working fine!

I tested the following:

  • Multiple filter selection
  • Max filter selection
  • Having old values in the localstorage
  • URL
  • Selector view

Left some small comments 👍

x-pack/plugins/cases/public/components/all_cases/multi_select_filter.tsx Outdated Show resolved Hide resolved
x-pack/plugins/cases/public/components/all_cases/multi_select_filter.test.tsx Outdated Show resolved Hide resolved
x-pack/plugins/cases/public/components/all_cases/multi_select_filter.test.tsx Show resolved Hide resolved
x-pack/plugins/cases/public/components/all_cases/status_filter.tsx Show resolved Hide resolved
x-pack/plugins/cases/public/components/all_cases/use_all_cases_state.test.tsx Outdated Show resolved Hide resolved
x-pack/plugins/cases/public/components/all_cases/utils/serialize_url_params.tsx Outdated Show resolved Hide resolved
x-pack/plugins/cases/public/components/all_cases/table_filters.tsx
@@ -222,23 +158,23 @@ const CasesTableFiltersComponent = ({
onSelectionChange={handleSelectedAssignees}
/>
) : null}
<FilterPopover
<MultiSelectFilter
Copy link
Contributor

Choose a reason for hiding this comment

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

I was testing PR and just realized that this is the only filter that we display in the selector view where the corresponding column is not visible.

If we filter by multiple tags there is no way of knowing which case has which.

Does this make sense? Screenshot 2023-11-07 at 11 58 24

@mdefazio @cnasikas

Copy link
Member

Choose a reason for hiding this comment

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

Yes, we decided that a couple of releases ago to reduce the space. The same occurs with the Solution filter. @jcger That reminded me that we did not convert the filter to the new component. Should we do it? (on another PR)

Screenshot 2023-11-07 at 1 53 30 PM

cnasikas
cnasikas approved these changes Nov 7, 2023
View reviewed changes
Copy link
Member

@cnasikas cnasikas left a comment

Choose a reason for hiding this comment

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

I tested with legacy filters (URL and local storage) and is working as expected. I left some minor comments.

x-pack/plugins/cases/public/components/all_cases/multi_select_filter.test.tsx Outdated Show resolved Hide resolved
x-pack/plugins/cases/public/components/all_cases/multi_select_filter.tsx Show resolved Hide resolved
x-pack/plugins/cases/public/components/all_cases/multi_select_filter.test.tsx Show resolved Hide resolved
x-pack/plugins/cases/public/containers/api.ts Show resolved Hide resolved
x-pack/plugins/cases/public/components/all_cases/utils/sanitize_filter_options.tsx Show resolved Hide resolved
x-pack/plugins/cases/public/components/all_cases/utils/serialize_url_params.tsx Outdated Show resolved Hide resolved
x-pack/plugins/cases/public/components/all_cases/utils/serialize_url_params.tsx Outdated Show resolved Hide resolved
x-pack/plugins/cases/public/components/all_cases/utils/sanitize_filter_options.test.tsx Outdated Show resolved Hide resolved
x-pack/plugins/cases/public/components/all_cases/utils/sanitize_filter_options.tsx Outdated Show resolved Hide resolved
x-pack/plugins/cases/public/components/all_cases/utils/sanitize_filter_options.tsx Outdated Show resolved Hide resolved
js-jankisalvi
js-jankisalvi approved these changes Nov 7, 2023
View reviewed changes
Copy link
Contributor

@js-jankisalvi js-jankisalvi left a comment

Choose a reason for hiding this comment

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

Verified locally, works as expected 👍 Nice work!! 🎉

@jcger jcger merged commit 36d8eba into elastic:cases/167651 Nov 8, 2023
1 check passed
@kibana-ci
Copy link
Collaborator

💔 Build Failed

Failed CI Steps

Metrics [docs]

‼️ ERROR: no builds found for mergeBase sha [b5cb887]

History

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

@jcger jcger mentioned this pull request Nov 30, 2023
Merged
jcger added a commit that referenced this pull request Dec 4, 2023
@jcger @adcoelho @kibanamachine
[Cases] Cases Table: Configure Available Filters Including Custom Fie…
90d6358 
…lds (#172276)

Meta issue #167651
Fixes: #167651

## Summary
Previous PRs merged into this feature branch:
- #169356
- #169371
- #170851
- #171102
- #171176

## Release notes
Case list filter bar can now be customised. Filters can be removed and
custom fields can be used as filters

## Pending issues
- Table in modal shouldn’t load in local storage saved filter options of
status/severity
- Status & Severity filters in url. Filters must be activated if the
user has them deactivated
- UI overflow when to much filters are active
- Race condition: When a user has a custom field active with an option
selected and this custom field gets removed in settings, it includes the
removed custom field when refreshing. This request will fail, triggering
a second one which won't include the removed custom field
- Found during QA. In the modal, when trying to select all options in
the solutions filter, when checking the last unchecked option, it resets
and there is no checked option anymore

## Flaky test runner link

https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4128

---------

Co-authored-by: Antonio <[email protected]>
Co-authored-by: kibanamachine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cnasikas cnasikas cnasikas approved these changes

@adcoelho adcoelho adcoelho approved these changes

@js-jankisalvi js-jankisalvi js-jankisalvi approved these changes

Assignees
No one assigned
Labels
Feature:Cases Cases feature release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.12.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@jcger @elasticmachine @kibana-ci @adcoelho @cnasikas @js-jankisalvi @kibanamachine