Add search functionality to filter Kafka clusters in UI#1894
Conversation
kapybro
Bot
added
status/triage/manual
|
AI Summary The issue describes adding a search function to the Kafka UI that allows users to filter and find Kafka clusters in real time from the navigation component. The implementation includes backend changes to support cluster filtering, API updates to define search parameters, and frontend additions for the UI and styling. Reviewers are asked to focus on search behavior with special characters, API backward compatibility, performance with large cluster lists, and correct ACL service filtering. |
kapybro
Bot
changed the title
📝 WalkthroughWalkthroughThree independent changes: ChangesACL Listing Feature Gate
OpenAPI OAuth Config Extensions
Nav Sidebar Cluster Search
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
kapybro
Bot
added
area/ux
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hi Akshaya-T! 👋
Welcome, and thank you for opening your first PR in the repo!
Please wait for triaging by our maintainers.
Please take a look at our contributing guide.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/src/components/Nav/Nav.tsx (1)
13-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd Nav search behavior tests (filter + no-results).
This PR adds new UI behavior, but current Nav tests shown in
frontend/src/components/Nav/__tests__/Nav.spec.tsx:16-42only cover base rendering. Add search filtering cases to lock behavior.🧪 Suggested test additions
+it('filters clusters by search query', () => { + renderComponent([onlineClusterPayload, offlineClusterPayload]); + fireEvent.change(screen.getByPlaceholderText('Search clusters...'), { + target: { value: onlineClusterPayload.name }, + }); + expect(screen.getByText(onlineClusterPayload.name)).toBeInTheDocument(); + expect(screen.queryByText(offlineClusterPayload.name)).not.toBeInTheDocument(); +}); + +it('shows no cluster menus when search has no matches', () => { + renderComponent([onlineClusterPayload, offlineClusterPayload]); + fireEvent.change(screen.getByPlaceholderText('Search clusters...'), { + target: { value: '__no_match__' }, + }); + expect(screen.queryByText(onlineClusterPayload.name)).not.toBeInTheDocument(); + expect(screen.queryByText(offlineClusterPayload.name)).not.toBeInTheDocument(); +});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/Nav/Nav.tsx` around lines 13 - 37, Add tests for the new Nav search behavior in Nav.spec.tsx by covering both filtering and empty-state cases. Use the Nav component’s search input to verify that typing a cluster name updates the rendered list to only matching clusters via the filteredClusters logic, and add a case where the search term matches nothing so the list renders no cluster items. Keep the existing base rendering tests and extend them around the Search input and cluster list assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/Nav/Nav.tsx`:
- Around line 15-20: The cluster filtering logic in Nav.tsx is checking
search.trim() but still matching names against the untrimmed search value, so
whitespace-padded queries can fail. Update the filteredClusters computation to
use a single trimmed search string for both the truthy check and the includes
comparison, keeping the logic in the Nav component and the search-based filter
consistent.
---
Nitpick comments:
In `@frontend/src/components/Nav/Nav.tsx`:
- Around line 13-37: Add tests for the new Nav search behavior in Nav.spec.tsx
by covering both filtering and empty-state cases. Use the Nav component’s search
input to verify that typing a cluster name updates the rendered list to only
matching clusters via the filteredClusters logic, and add a case where the
search term matches nothing so the list renders no cluster items. Keep the
existing base rendering tests and extend them around the Search input and
cluster list assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a62fd65e-758e-44b6-928a-1eb138578542
📒 Files selected for processing (4)
api/src/main/java/io/kafbat/ui/service/acl/AclsService.javaapi/src/main/resources/static/openapi/kafbat-ui-api.yamlfrontend/src/components/Nav/Nav.styled.tsfrontend/src/components/Nav/Nav.tsx
| const filteredClusters = | ||
| clusters.isSuccess && search.trim() | ||
| ? clusters.data.filter((c) => | ||
| c.name.toLowerCase().includes(search.toLowerCase()) | ||
| ) | ||
| : clusters.data; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Trim the search query before matching cluster names.
Line 16 checks search.trim(), but Line 18 matches with untrimmed search, so queries like " prod " can miss valid matches.
💡 Proposed fix
- const filteredClusters =
- clusters.isSuccess && search.trim()
+ const normalizedSearch = search.trim().toLowerCase();
+ const filteredClusters =
+ clusters.isSuccess && normalizedSearch
? clusters.data.filter((c) =>
- c.name.toLowerCase().includes(search.toLowerCase())
+ c.name.toLowerCase().includes(normalizedSearch)
)
: clusters.data;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const filteredClusters = | |
| clusters.isSuccess && search.trim() | |
| ? clusters.data.filter((c) => | |
| c.name.toLowerCase().includes(search.toLowerCase()) | |
| ) | |
| : clusters.data; | |
| const normalizedSearch = search.trim().toLowerCase(); | |
| const filteredClusters = | |
| clusters.isSuccess && normalizedSearch | |
| ? clusters.data.filter((c) => | |
| c.name.toLowerCase().includes(normalizedSearch) | |
| ) | |
| : clusters.data; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/Nav/Nav.tsx` around lines 15 - 20, The cluster
filtering logic in Nav.tsx is checking search.trim() but still matching names
against the untrimmed search value, so whitespace-padded queries can fail.
Update the filteredClusters computation to use a single trimmed search string
for both the truthy check and the includes comparison, keeping the logic in the
Nav component and the search-based filter consistent.
1 participant
What changes did you make?
Added a search/filter function in the Kafka UI to let users quickly filter and find Kafka clusters. This change improves usability for users managing multiple clusters by providing real-time search directly in the navigation component.
Key Changes:
api/src/main/java/io/kafbat/ui/service/acl/AclsService.javato support cluster filtering logic.api/src/main/resources/static/openapi/kafbat-ui-api.yamlwith search/filter request parameters and response definitions.frontend/src/components/Nav/Nav.tsx.frontend/src/components/Nav/Nav.styled.ts.Is there anything you'd like reviewers to focus on?
How Has This Been Tested? (put an "x" (case-sensitive!) next to an item)
Tested manually by filtering multiple Kafka cluster entries through the navigation search UI and confirming results matched expected cluster names.
Checklist (put an "x" (case-sensitive!) next to all the items, otherwise the build will fail)
Summary by CodeRabbit
New Features
Bug Fixes
Documentation