feat: limit badges to 2 with hover/click popover in UserListTable#8
Conversation
Code Review by Qodo
1. handleDownload throws generic Error
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+581
to
584
| const handleDownload = async (): Promise<void> => { | ||
| try { | ||
| if (!org?.slug || !org?.name) { | ||
| throw new Error("Org slug or name is missing."); |
There was a problem hiding this comment.
1. handledownload throws generic error 📘 Rule violation ⛯ Reliability
• handleDownload uses multiple throw new Error(...) statements for expected failure modes (missing org fields, missing headers, empty results, CSV generation failure). • This violates the requirement to use hierarchical custom error classes with typed codes and (where applicable) HTTP status codes, reducing debuggability and consistent handling. • It also makes it harder to programmatically differentiate user-correctable issues vs system failures.
Agent prompt
## Issue description
`handleDownload` throws generic `Error` objects for multiple failure cases. Compliance requires hierarchical custom errors with typed codes (and status codes for HTTP/API errors) plus structured context.
## Issue Context
This function covers a CSV export flow with several expected validation/failure branches (missing org data, missing headers, empty result set, CSV generation failure). These should be represented with typed custom errors to enable consistent handling and debugging.
## Fix Focus Areas
- apps/web/modules/users/components/UserTable/UserListTable.tsx[581-622]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+91
to
+98
| <div className="flex flex-col gap-1"> | ||
| {hiddenItems.map((item, index) => ( | ||
| <span | ||
| key={item.label} | ||
| className="text-default cursor-pointer text-sm hover:text-emphasis"> | ||
| {item.label} | ||
| </span> | ||
| ))} |
There was a problem hiding this comment.
3. Hidden items not clickable 🐞 Bug ✓ Correctness
• LimitedBadges accepts per-item onClick, but renders overflow (hidden) items as plain <span> without binding item.onClick, so clicks do nothing. • UserListTable passes onClick handlers for team badges to filter the table; for users in >2 teams, hidden teams in the popover cannot be clicked to filter. • Hidden items are styled with cursor-pointer, increasing user confusion because they look interactive but aren’t.
Agent prompt
### Issue description
Hidden (overflow) items in `LimitedBadges` are rendered as non-interactive `<span>` elements. This ignores `BadgeItem.onClick`, causing functional regressions (e.g., team filtering in `UserListTable` for users with >2 teams).
### Issue Context
`UserListTable` passes `onClick` handlers to `LimitedBadges` items (teams column) expecting filter changes when clicking team names. Only the first `maxVisible` items are actually clickable today.
### Fix Focus Areas
- apps/web/components/ui/LimitedBadges.tsx[66-101]
- apps/web/modules/users/components/UserTable/UserListTable.tsx[385-393]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+68
to
+72
| {visibleItems.map((item, index) => ( | ||
| <Badge key={item.label} variant={item.variant || "gray"} onClick={item.onClick}> | ||
| {item.label} | ||
| </Badge> | ||
| ))} |
There was a problem hiding this comment.
4. Label-based key collisions 🐞 Bug ⛯ Reliability
• LimitedBadges uses item.label as the React key for both visible and hidden lists; labels are not guaranteed unique. • In insights, multi-select values are typed as an arbitrary string array (no uniqueness constraint), and labels are derived from optionMap, so duplicate keys can occur. • Duplicate keys can cause React reconciliation bugs (wrong item reused) and runtime warnings, especially in dense table UIs.
Agent prompt
### Issue description
`LimitedBadges` uses `item.label` as the React list key. Labels are not guaranteed unique (and multi-select values are not unique-enforced), which can produce duplicate keys and reconciliation issues.
### Issue Context
- Response values are validated as `z.array(z.string())` (no uniqueness constraint).
- `ResponseValueCell` labels are derived via `optionMap[id] ?? id`, and `optionMap` construction does not enforce unique labels.
### Fix Focus Areas
- apps/web/components/ui/LimitedBadges.tsx[11-26]
- apps/web/components/ui/LimitedBadges.tsx[66-98]
- apps/web/modules/insights/components/ResponseValueCell.tsx[15-21]
- apps/web/modules/users/components/UserTable/UserListTable.tsx[239-266]
- apps/web/modules/users/components/UserTable/UserListTable.tsx[385-393]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Benchmark PR from agentic-review-benchmarks#8