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] UI validation for assignees, tags and categories filters #162411

Merged
merged 7 commits into from
Jul 26, 2023

Conversation

js-jankisalvi
Copy link
Contributor

@js-jankisalvi js-jankisalvi commented Jul 24, 2023

Summary

Connected to #146945

This PR adds UI validations for assignees, tags and categories filter on cases list table and cases selector modal:

Description Limit Done? Documented? UI?
Maximum number of assignees to filter 100 Yes
Maximum number of tags to filter 100 Yes
Maximum number of categories to filter 100 Yes

Selector modal:

image

Case list table:

image

image

image

Note: screenshots are taken with 5 as maximum limit for assignees, tags and categories filter:

Checklist

Delete any items that are not applicable to this PR.

For maintainers

Release notes

The Find Cases API now limits

  • Maximum number of assignees to filter to 100
  • Maximum number of tags to filter to 100
  • Maximum number of categories to filter to 100

@js-jankisalvi js-jankisalvi added release_note:breaking Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:Cases Cases feature v8.10.0 labels Jul 24, 2023
@js-jankisalvi js-jankisalvi requested a review from a team as a code owner July 24, 2023 14:06
@js-jankisalvi js-jankisalvi self-assigned this Jul 24, 2023
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

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

@js-jankisalvi js-jankisalvi changed the title UI validation for filters [Cases] UI validation for assignees, tags and categories filters Jul 24, 2023
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.

Creat job 🚀!!

appMockRender.render(<AssigneesFilterPopover {...props} />);

await waitFor(async () => {
userEvent.click(screen.getByTestId('options-filter-popover-button-assignees'));
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: we use the old "@testing-library/user-event": "^13.5.0", which doesn't return a promise(I think it is here) so this could be moved outside.

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 tried to move userEvent.click(screen.getByTestId('options-filter-popover-button-assignees')); out of waitFor but test fails due to it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Weird, in the documentation for this version they don't use it. I don't know what to believe anymore 😆

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, we also don't put it in waitFor in other components tests. Maybe something wrong in rendering this component.

Copy link
Member

@cnasikas cnasikas Jul 27, 2023

Choose a reason for hiding this comment

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

FWIW, we can convert it to userEvent.click(await screen.findByTestId('options-filter-popover-button-assignees')); and remove the waitFor. All flavors of find* await for the element to appear.

Copy link
Contributor

@adcoelho adcoelho left a comment

Choose a reason for hiding this comment

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

Looks great!

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
cases 422.1KB 422.7KB +551.0B

Page load bundle

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

id before after diff
cases 147.6KB 147.9KB +255.0B

History

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

cc @js-jankisalvi

@js-jankisalvi js-jankisalvi merged commit 22dc782 into elastic:main Jul 26, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jul 26, 2023
ThomThomson pushed a commit to ThomThomson/kibana that referenced this pull request Aug 1, 2023
…stic#162411)

## Summary

Connected to elastic#146945

This PR adds UI validations for `assignees`, `tags` and `categories`
filter on cases list table and cases selector modal:

Description | Limit | Done? | Documented? | UI?
-- | -- | -- | -- | --
Maximum number of assignees to filter | 100 | ✅ | Yes |
:white_check_mark:
Maximum number of tags to filter | 100 | ✅ | Yes | :white_check_mark:
Maximum number of categories to filter | 100 | ✅ | Yes |
:white_check_mark:

**Selector modal:** 


![image](https://github.com/elastic/kibana/assets/117571355/69945b0a-57af-42c0-85e0-7df497d8796b)

**Case list table:** 


![image](https://github.com/elastic/kibana/assets/117571355/05c882f8-c160-40c3-aa9c-70ad4801e837)


![image](https://github.com/elastic/kibana/assets/117571355/e8e3eef8-81cf-46a2-8c8c-ee0d1f65a8ec)


![image](https://github.com/elastic/kibana/assets/117571355/a30bd780-d36f-437f-bf29-6eafed6accca)

_Note:_ _screenshots are taken with 5 as maximum limit for `assignees`,
`tags` and `categories` filter:_

### 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/))

### 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)
@js-jankisalvi js-jankisalvi deleted the ui-validation-for-filters branch February 19, 2024 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Cases Cases feature release_note:breaking Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants