-
Notifications
You must be signed in to change notification settings - Fork 116
Fix issues with basic visibility filters #928
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
base: master
Are you sure you want to change the base?
Fix issues with basic visibility filters #928
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes filter visibility logic in the basic workflows view and corrects time-range parameters in the advanced workflows view.
- In the advanced view, use the correct
timeRangeStart
/timeRangeEnd
query keys and update the effect dependencies. - In the basic view, integrate the shared
usePageFilters
hook to combine manual search filters with page filters, pass anareAnyFiltersActive
flag to the table, and update error-panel logic/tests accordingly.
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
src/views/domain-workflows-advanced/domain-workflows-advanced.tsx | Switched from timeRangeStartBasic /timeRangeEndBasic to generic query keys and updated dependencies. |
src/views/domain-workflows-basic/domain-workflows-basic.tsx | Added usePageFilters hook, wired props into filters and table. |
src/views/domain-workflows-basic/domain-workflows-basic-table/helpers/get-workflows-basic-error-panel-props.ts | Renamed parameter to areAnyFiltersActive and adjusted logic. |
src/views/domain-workflows-basic/domain-workflows-basic-table/domain-workflows-basic-table.types.ts | Added new areAnyFiltersActive prop. |
src/views/domain-workflows-basic/domain-workflows-basic-table/domain-workflows-basic-table.tsx | Removed old query-param hook, now uses the passed-in filters flag. |
src/views/domain-workflows-basic/domain-workflows-basic-filters/domain-workflows-basic-filters.types.ts | Introduced a Props type based on usePageFilters return value. |
src/views/domain-workflows-basic/domain-workflows-basic-filters/domain-workflows-basic-filters.tsx | Switched the filters component to accept injected usePageFilters props. |
Multiple test files | Updated tests to pass the new flag and renamed parameter in tests. |
Comments suppressed due to low confidence (3)
src/views/domain-workflows-basic/domain-workflows-basic-table/domain-workflows-basic-table.types.ts:7
- [nitpick] Consider renaming
areAnyFiltersActive
tohasActiveFilters
orisFilterActive
for improved readability and consistency with boolean naming conventions.
areAnyFiltersActive: boolean;
src/views/domain-workflows-basic/domain-workflows-basic-filters/domain-workflows-basic-filters.tsx:5
- The component uses
domainPageQueryParamsConfig
inPageFiltersSearch
but there’s no import for it; addimport domainPageQueryParamsConfig from '../domain-page/config/domain-page-query-params.config';
.
import PageFiltersSearch from '@/components/page-filters/page-filters-search/page-filters-search';
src/views/domain-workflows-basic/domain-workflows-basic-filters/domain-workflows-basic-filters.types.ts:1
- You cannot use
typeof usePageFilters
on a type-only import; either import the hook as a value (import usePageFilters from ...
) or change the Props type toReturnType<usePageFilters>
withouttypeof
.
import type usePageFilters from '@/components/page-filters/hooks/use-page-filters';
nice Catch |
Summary
areSearchParamsAbsent
forgetWorkflowsBasicErrorPanelProps
tohasActiveSearchParams
Test plan
Updated unit tests + ran locally.
Before
After