-
Notifications
You must be signed in to change notification settings - Fork 17
fix: problem filter to query #3047
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
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 refactors the problem filter functionality from Redux state management to URL query parameters using the use-query-params library. The changes remove centralized Redux state for filtering and replace it with URL-based state management, making the filter state shareable via URLs and more aligned with the project's move towards use-query-params.
- Removed
problemFilterfrom Redux settings state and replaced with URL-basedwithProblemsboolean parameter - Migrated tenants filtering logic from Redux selectors to local component state with filter utility functions
- Updated
ProblemFiltercomponent to work with boolean values instead of string enums and added i18n support
Reviewed Changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/nodes.ts | Removed getProblemParamValue helper function no longer needed after refactoring |
| src/utils/hooks/useWithProblemsQueryParam.ts | New hook to manage withProblems URL parameter with BooleanParam |
| src/store/state-url-mapping.ts | Removed problemFilter from global redux-location-state params and cleaned up unused tenants search mapping |
| src/store/reducers/tenants/types.ts | Removed TenantsState and TenantsStateSlice interfaces as tenants state is no longer in Redux |
| src/store/reducers/tenants/tenants.ts | Removed Redux slice for tenants, keeping only the RTK Query API endpoints |
| src/store/reducers/tenants/selectors.ts | Deleted file - filtering logic migrated to new filters.ts utility file |
| src/store/reducers/tenants/filters.ts | New file with pure filtering functions for tenants (by problems, search, domain) |
| src/store/reducers/settings/types.ts | Removed ProblemFilterValue type and problemFilter from SettingsState |
| src/store/reducers/settings/settings.ts | Removed ProblemFilterValues constant, changeFilter action, and selectProblemFilter selector |
| src/store/reducers/settings/hooks.ts | Deleted file - useProblemFilter hook replaced by URL param hooks |
| src/store/reducers/nodes/types.ts | Changed problemFilter field to withProblems boolean in NodesFilters interface |
| src/store/reducers/index.ts | Removed tenants reducer from root reducer as it no longer manages Redux state |
| src/containers/Tenants/useTenantsQueryParams.ts | New hook managing both withProblems and search URL parameters for Tenants page |
| src/containers/Tenants/Tenants.tsx | Refactored to use URL params instead of Redux, with local useMemo for filtering logic |
| src/containers/Tenant/Diagnostics/Network/Network.tsx | Updated to use useWithProblemsQueryParam hook instead of Redux state |
| src/containers/Nodes/useNodesPageQueryParams.ts | Added withProblems BooleanParam to existing query params management |
| src/containers/Nodes/getNodes.ts | Changed parameter from problemFilter to withProblems boolean |
| src/containers/Nodes/PaginatedNodes/PaginatedNodes.tsx | Updated to use withProblems from URL params instead of Redux |
| src/containers/Nodes/PaginatedNodes/NodesComponent.tsx | Updated to destructure withProblems from useNodesPageQueryParams |
| src/containers/Nodes/PaginatedNodes/GroupedNodesComponent.tsx | Changed hardcoded 'All' to false for withProblems parameter |
| src/containers/Nodes/NodesTable.tsx | Changed prop type from ProblemFilterValue to boolean |
| src/containers/Nodes/NodesControls/NodesControls.tsx | Updated to use URL param hook instead of Redux for problem filter |
| src/components/ProblemFilter/index.ts | Deleted barrel export file |
| src/components/ProblemFilter/i18n/index.ts | New i18n configuration for ProblemFilter component |
| src/components/ProblemFilter/i18n/en.json | New i18n translations for "all" and "with-problems" options |
| src/components/ProblemFilter/ProblemFilter.tsx | Refactored to use boolean value and onChange, with string-to-boolean conversion and i18n |
Comments suppressed due to low confidence (1)
src/containers/Tenant/Diagnostics/Network/Network.tsx:96
- The Checkbox
onUpdatecallbacks should be memoized usingReact.useCallbackaccording to the project's mandatory React performance patterns. All event handlers should be wrapped withuseCallback:
const handleShowIdChange = React.useCallback(() => {
setShowId(!showId);
}, [showId]);
const handleShowRacksChange = React.useCallback(() => {
setShowRacks(!showRacks);
}, [showRacks]);Then use them:
<Checkbox onUpdate={handleShowIdChange} checked={showId}>
ID
</Checkbox>
<Checkbox onUpdate={handleShowRacksChange} checked={showRacks}>
Racks
</Checkbox> <div className={b('checkbox-wrapper')}>
<Checkbox
onUpdate={() => {
setShowId(!showId);
}}
checked={showId}
>
ID
</Checkbox>
</div>
<div className={b('checkbox-wrapper')}>
<Checkbox
onUpdate={() => {
setShowRacks(!showRacks);
}}
checked={showRacks}
>
Racks
</Checkbox>
</div>
| @@ -0,0 +1,4 @@ | |||
| { | |||
| "all": "All", | |||
| "with-problems": "With problems" | |||
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.
i18n namings should meet this ruleset https://github.com/ydb-platform/ydb-embedded-ui/blob/main/i18n-naming-ruleset.md
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.
26 files reviewed, no comments
Part of #2892
Stand: https://nda.ya.ru/t/c9IJGZLG7MWD5i
CI Results
Test Status:⚠️ FLAKY
📊 Full Report
Test Changes Summary ⏭️2
⏭️ Skipped Tests (2)
Bundle Size: 🔽
Current: 47.15 MB | Main: 47.15 MB
Diff: 3.05 KB (-0.01%)
✅ Bundle size decreased.
ℹ️ CI Information