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

[Issue #2144] e2e tests for clear/select all filters and fix nested filter bug #3319

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

doug-s-nava
Copy link
Collaborator

@doug-s-nava doug-s-nava commented Dec 19, 2024

Summary

Fixes #2144

Time to review: 15 mins

Changes proposed

  • adds e2e tests for "select all" and "clear all" filter functionality, including for agencies / nested filters
  • fixes bugs with nested filters that showed incorrect selected filter counts and did not correctly clear selected filters

Context for reviewers

Test steps

Observe bug

  1. start the app on main
  2. from the search page, click "select all" at the top level of the agencies filter
  3. VALIDATE: the ui suggests that 39 items are selected, when only 21 are
  4. open an expander to show nested filter options
  5. click "select all" for the nested options
  6. VALIDATE: top level search options are cleared when nested ones are selected
  7. select a top level option
  8. click "clear all" under nested section
  9. VALIDATE: all options are cleared, not just nested ones

Validate fix

  1. start the app on this branch
  2. from the search page, click "select all" at the top level of the agencies filter
  3. VALIDATE: the ui states that 21 items are selected
  4. open an expander to show nested filter options
  5. click "select all" for the nested options
  6. VALIDATE: top level search options are not cleared
  7. select a top level option
  8. click "clear all" under nested section
  9. VALIDATE: only nested options are cleared

Additional information

The filter components are still a bit of a mess, made a follow up ticket for some more cleanup work. #3331

@doug-s-nava doug-s-nava force-pushed the dschrashun/2144-e2e-clear-filters branch from 7b405e6 to 906ac8e Compare December 20, 2024 15:38
@doug-s-nava doug-s-nava force-pushed the dschrashun/2144-e2e-clear-filters branch from 906ac8e to 7c3773e Compare December 20, 2024 15:43
@doug-s-nava doug-s-nava marked this pull request as ready for review December 20, 2024 16:08
@@ -43,20 +44,42 @@ export interface FilterOptionWithChildren {
children: FilterOption[];
}

const isSectionAllSelected = (
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

these should probably be moved to a utils file, but getting them out of the component is a good first step. Saving bigger refactors for #3331

Copy link
Collaborator

@acouch acouch left a comment

Choose a reason for hiding this comment

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

👍🏿

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add e2e test that covers select all/clear all filter options
2 participants