Group tabs by users when multiple usernames are configured#31
Open
kertal wants to merge 2 commits into
Open
Conversation
When multiple users are entered (comma-separated), all three views (Summary, Issues & PRs, Events) now group their content by user with collapsible user sections showing avatar, username, and item count. Deduplication is skipped in multi-user mode to preserve per-user items that may share the same URL across different users' activity. Single-user mode behavior is completely unchanged. https://claude.ai/code/session_01CKTzchrxuW2rxEjK9Aq5FK
Adds a "Group by users" toggle switch next to the tab navigation that only appears when multiple usernames are configured. The toggle is persisted in localStorage. When off (default), multi-user mode behaves the same as before — all items are shown in a single flat view. https://claude.ai/code/session_01CKTzchrxuW2rxEjK9Aq5FK
📝 WalkthroughWalkthroughThis pull request introduces multi-user support throughout the application. It adds a Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant App as App.tsx
participant Form as SearchForm.tsx
participant Hook as useGitHubDataProcessing
participant Utils as rawDataUtils.ts
participant Views as Views<br/>(EventView, etc.)
User->>Form: Toggle "Group by Users"
Form->>App: setGroupByUsers(true)
App->>Hook: Passes isMultiUser & groupByUsers
Hook->>Utils: categorizeRawSearchItems(items, ..., skipDedup=true)
Utils-->>Hook: Returns undeduped items by user
Hook->>Hook: Compute perUserGroupedItems
Hook-->>Views: Pass grouped data
Views->>Views: groupItemsByUser() sorts by configured users
Views->>Views: Render per-user sections with avatars & toggles
Views-->>User: Display items grouped by user login
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
src/views/Summary.tsx (2)
72-110: Duplicate code:groupItemsByUseris repeated across views.Same function appears in
IssuesAndPRsList.tsxandEventView.tsx. As noted in the IssuesAndPRsList review, this should be extracted to a shared utility.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/views/Summary.tsx` around lines 72 - 110, Extract the duplicated groupItemsByUser function into a shared utility module (e.g., export a function named groupItemsByUser from a new utils file), keep the same signature (items: GitHubItem[], configuredUsernames: string[]) and behavior, export any needed types (or import them into the util) and then replace the local implementations in Summary.tsx, IssuesAndPRsList.tsx, and EventView.tsx with a single import of the shared groupItemsByUser; ensure all three files import the function and that TypeScript types still resolve correctly.
150-166: Redundant re-ordering logic.The ordering logic here duplicates what
groupItemsByUseralready does. SincegroupItemsByUserpreserves configured user order, you could simplify this by applyinggroupItemsByUserto the union of users earlier.♻️ Consider simplifying
Since
groupItemsByUseralready orders users correctly, consider restructuring to avoid the second ordering pass:const perUserActionGroups = useMemo((): UserActionGroups | null => { if (!showUserGroups) return null; // Use groupItemsByUser for ordering at the start const userItems = groupItemsByUser(sortedItems, usernames); const userSearchItems = groupItemsByUser(filteredIndexedDBSearchItems, usernames); const result: Record<string, Record<string, GitHubItem[]>> = {}; // Process in the order returned by groupItemsByUser (already ordered) for (const login of Object.keys(userItems)) { result[login] = groupSummaryData( userItems[login] || [], userSearchItems[login] || [], startDate, endDate ); } // Add any users only in search items for (const login of Object.keys(userSearchItems)) { if (!result[login]) { result[login] = groupSummaryData( [], userSearchItems[login], startDate, endDate ); } } return result; }, [showUserGroups, sortedItems, filteredIndexedDBSearchItems, usernames, startDate, endDate]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/views/Summary.tsx` around lines 150 - 166, The current reorder pass after building `result` is redundant because `groupItemsByUser` already preserves configured `usernames`; refactor the `perUserActionGroups` logic to call `groupItemsByUser(sortedItems, usernames)` and `groupItemsByUser(filteredIndexedDBSearchItems, usernames)` first, then iterate the keys returned by those grouped maps (in that order) to call `groupSummaryData` and assemble `result`, adding any additional keys from the search grouping that aren’t present yet; remove the subsequent manual re-ordering block that computes `configuredLower`, `ordered`, and the filter/sort/forEach so the ordering comes solely from `groupItemsByUser`.src/views/EventView.tsx (1)
41-76: Duplicate code:groupItemsByUserappears here as well.This is the third copy of the same function. Please consolidate with the extraction suggested in the IssuesAndPRsList.tsx review.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/views/EventView.tsx` around lines 41 - 76, The function groupItemsByUser is duplicated; extract it into a single shared utility (e.g., export from a new or existing helper module) and replace the copies in EventView (the groupItemsByUser declaration) and the other component reviewed (IssuesAndPRsList) by importing and using that shared function; ensure the exported signature matches (items: GitHubItem[], configuredUsernames: string[]) => Record<string, GitHubItem[]> and update any references to rely on the imported symbol, removing the local duplicate implementations.src/views/IssuesAndPRsList.tsx (1)
113-148: ExtractgroupItemsByUserto a shared utility.This function is duplicated verbatim in
IssuesAndPRsList.tsx,Summary.tsx, andEventView.tsx. Consider extracting it tosrc/utils/viewFiltering.tsor a newsrc/utils/userGrouping.tsto eliminate duplication and ensure consistent behavior across views.♻️ Proposed refactor
Create a shared utility:
// src/utils/userGrouping.ts import { GitHubItem } from '../types'; /** Groups items by user login, preserving configured user order. */ export const groupItemsByUser = ( items: GitHubItem[], configuredUsernames: string[] ): Record<string, GitHubItem[]> => { const userGroups: Record<string, GitHubItem[]> = {}; const configuredLower = configuredUsernames.map(u => u.toLowerCase()); items.forEach(item => { const login = item.user.login; if (!userGroups[login]) { userGroups[login] = []; } userGroups[login].push(item); }); const ordered: Record<string, GitHubItem[]> = {}; for (const configUser of configuredUsernames) { const matchKey = Object.keys(userGroups).find( k => k.toLowerCase() === configUser.toLowerCase() ); if (matchKey && userGroups[matchKey].length > 0) { ordered[matchKey] = userGroups[matchKey]; } } Object.keys(userGroups) .filter(k => !configuredLower.includes(k.toLowerCase())) .sort((a, b) => a.toLowerCase().localeCompare(b.toLowerCase())) .forEach(k => { if (userGroups[k].length > 0) { ordered[k] = userGroups[k]; } }); return ordered; };Then import in each view file:
+import { groupItemsByUser } from '../utils/userGrouping'; -/** Groups items by user login, preserving configured user order. */ -const groupItemsByUser = ( ... );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/views/IssuesAndPRsList.tsx` around lines 113 - 148, Duplicate groupItemsByUser implementations exist across IssuesAndPRsList, Summary, and EventView; extract the function into a single exported utility (e.g., userGrouping.ts) and update each component to import and use that exported groupItemsByUser. Ensure the exported function signature and behavior exactly match the current groupItemsByUser (items: GitHubItem[], configuredUsernames: string[]) and remove the duplicated local definitions from IssuesAndPRsList, Summary, and EventView so they reference the shared utility instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/views/EventView.tsx`:
- Around line 41-76: The function groupItemsByUser is duplicated; extract it
into a single shared utility (e.g., export from a new or existing helper module)
and replace the copies in EventView (the groupItemsByUser declaration) and the
other component reviewed (IssuesAndPRsList) by importing and using that shared
function; ensure the exported signature matches (items: GitHubItem[],
configuredUsernames: string[]) => Record<string, GitHubItem[]> and update any
references to rely on the imported symbol, removing the local duplicate
implementations.
In `@src/views/IssuesAndPRsList.tsx`:
- Around line 113-148: Duplicate groupItemsByUser implementations exist across
IssuesAndPRsList, Summary, and EventView; extract the function into a single
exported utility (e.g., userGrouping.ts) and update each component to import and
use that exported groupItemsByUser. Ensure the exported function signature and
behavior exactly match the current groupItemsByUser (items: GitHubItem[],
configuredUsernames: string[]) and remove the duplicated local definitions from
IssuesAndPRsList, Summary, and EventView so they reference the shared utility
instead.
In `@src/views/Summary.tsx`:
- Around line 72-110: Extract the duplicated groupItemsByUser function into a
shared utility module (e.g., export a function named groupItemsByUser from a new
utils file), keep the same signature (items: GitHubItem[], configuredUsernames:
string[]) and behavior, export any needed types (or import them into the util)
and then replace the local implementations in Summary.tsx, IssuesAndPRsList.tsx,
and EventView.tsx with a single import of the shared groupItemsByUser; ensure
all three files import the function and that TypeScript types still resolve
correctly.
- Around line 150-166: The current reorder pass after building `result` is
redundant because `groupItemsByUser` already preserves configured `usernames`;
refactor the `perUserActionGroups` logic to call `groupItemsByUser(sortedItems,
usernames)` and `groupItemsByUser(filteredIndexedDBSearchItems, usernames)`
first, then iterate the keys returned by those grouped maps (in that order) to
call `groupSummaryData` and assemble `result`, adding any additional keys from
the search grouping that aren’t present yet; remove the subsequent manual
re-ordering block that computes `configuredLower`, `ordered`, and the
filter/sort/forEach so the ordering comes solely from `groupItemsByUser`.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds multi-user grouping functionality to all three main views (Summary, Issues & PRs, Events) when multiple comma-separated usernames are configured. The feature allows users to toggle grouping on/off via a new "Group by users" switch that appears when multiple usernames are detected.
Changes:
- Added toggle switch in SearchForm to control user grouping (only visible with multiple usernames)
- Modified data processing to skip deduplication when multi-user grouping is enabled
- Implemented user-grouped rendering across all three view components with collapsible user sections
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/App.tsx | Added context fields for multi-user detection and grouping toggle state |
| src/components/SearchForm.tsx | Added toggle switch UI for "Group by users" option |
| src/hooks/useGitHubDataProcessing.ts | Modified to conditionally skip deduplication in multi-user mode |
| src/utils/rawDataUtils.ts | Added skipDedup parameter to categorizeRawSearchItems function |
| src/views/Summary.tsx | Implemented user grouping with avatar headers and nested action groups |
| src/views/IssuesAndPRsList.tsx | Implemented user grouping with PRs/Issues subgroups per user |
| src/views/EventView.tsx | Implemented user grouping with per-user pagination |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+150
to
+168
| // Re-order by configured usernames first | ||
| const configuredLower = usernames.map(u => u.toLowerCase()); | ||
| const ordered: Record<string, Record<string, GitHubItem[]>> = {}; | ||
| for (const configUser of usernames) { | ||
| const matchKey = Object.keys(result).find( | ||
| k => k.toLowerCase() === configUser.toLowerCase() | ||
| ); | ||
| if (matchKey) { | ||
| ordered[matchKey] = result[matchKey]; | ||
| } | ||
| } | ||
| Object.keys(result) | ||
| .filter(k => !configuredLower.includes(k.toLowerCase())) | ||
| .sort((a, b) => a.toLowerCase().localeCompare(b.toLowerCase())) | ||
| .forEach(k => { | ||
| ordered[k] = result[k]; | ||
| }); | ||
|
|
||
| return ordered; |
There was a problem hiding this comment.
The user ordering logic (configured users first, then others alphabetically) is duplicated twice within the same component: once inside groupItemsByUser (lines 88-109) and again in the perUserActionGroups useMemo (lines 150-166). Since groupItemsByUser already returns ordered results, the re-ordering in lines 150-166 is redundant and adds unnecessary computational overhead. Consider removing the duplicate ordering logic in the useMemo.
Suggested change
| // Re-order by configured usernames first | |
| const configuredLower = usernames.map(u => u.toLowerCase()); | |
| const ordered: Record<string, Record<string, GitHubItem[]>> = {}; | |
| for (const configUser of usernames) { | |
| const matchKey = Object.keys(result).find( | |
| k => k.toLowerCase() === configUser.toLowerCase() | |
| ); | |
| if (matchKey) { | |
| ordered[matchKey] = result[matchKey]; | |
| } | |
| } | |
| Object.keys(result) | |
| .filter(k => !configuredLower.includes(k.toLowerCase())) | |
| .sort((a, b) => a.toLowerCase().localeCompare(b.toLowerCase())) | |
| .forEach(k => { | |
| ordered[k] = result[k]; | |
| }); | |
| return ordered; | |
| return result; |
Comment on lines
+72
to
+110
| /** Groups items by user login, preserving configured user order. */ | ||
| const groupItemsByUser = ( | ||
| items: GitHubItem[], | ||
| configuredUsernames: string[] | ||
| ): Record<string, GitHubItem[]> => { | ||
| const userGroups: Record<string, GitHubItem[]> = {}; | ||
| const configuredLower = configuredUsernames.map(u => u.toLowerCase()); | ||
|
|
||
| items.forEach(item => { | ||
| const login = item.user.login; | ||
| if (!userGroups[login]) { | ||
| userGroups[login] = []; | ||
| } | ||
| userGroups[login].push(item); | ||
| }); | ||
|
|
||
| // Order: configured users first (in order), then others alphabetically | ||
| const ordered: Record<string, GitHubItem[]> = {}; | ||
| for (const configUser of configuredUsernames) { | ||
| // Find the actual case-sensitive key that matches | ||
| const matchKey = Object.keys(userGroups).find( | ||
| k => k.toLowerCase() === configUser.toLowerCase() | ||
| ); | ||
| if (matchKey && userGroups[matchKey].length > 0) { | ||
| ordered[matchKey] = userGroups[matchKey]; | ||
| } | ||
| } | ||
| // Add remaining users not in configured list | ||
| Object.keys(userGroups) | ||
| .filter(k => !configuredLower.includes(k.toLowerCase())) | ||
| .sort((a, b) => a.toLowerCase().localeCompare(b.toLowerCase())) | ||
| .forEach(k => { | ||
| if (userGroups[k].length > 0) { | ||
| ordered[k] = userGroups[k]; | ||
| } | ||
| }); | ||
|
|
||
| return ordered; | ||
| }; |
There was a problem hiding this comment.
The new multi-user grouping functionality lacks test coverage. The codebase has comprehensive test files for views (Summary.test.tsx, IssuesAndPRsList.test.tsx), but no tests have been added for the new groupItemsByUser function or the multi-user rendering logic. Consider adding tests to verify: 1) Correct ordering of configured users followed by alphabetical ordering of others, 2) Case-insensitive username matching, 3) Empty user groups are filtered out, 4) Collapse/expand behavior for user sections, and 5) Selection state management across user groups.
Comment on lines
+277
to
+285
| <Text sx={{ fontSize: 1, color: 'fg.muted', whiteSpace: 'nowrap' }}> | ||
| Group by users | ||
| </Text> | ||
| <ToggleSwitch | ||
| size="small" | ||
| checked={groupByUsers} | ||
| onChange={() => setGroupByUsers(!groupByUsers)} | ||
| aria-label="Group by users" | ||
| /> |
There was a problem hiding this comment.
The "Group by users" label (line 277-279) is visually associated with the ToggleSwitch but not semantically linked for screen readers. Consider wrapping both in a FormControl component with a proper label element, or adding an id to the Text and connecting it via aria-labelledby on the ToggleSwitch to improve accessibility for users relying on assistive technology.
Suggested change
| <Text sx={{ fontSize: 1, color: 'fg.muted', whiteSpace: 'nowrap' }}> | |
| Group by users | |
| </Text> | |
| <ToggleSwitch | |
| size="small" | |
| checked={groupByUsers} | |
| onChange={() => setGroupByUsers(!groupByUsers)} | |
| aria-label="Group by users" | |
| /> | |
| <FormControl> | |
| <FormControl.Label sx={{ fontSize: 1, color: 'fg.muted', whiteSpace: 'nowrap', mb: 0 }}> | |
| Group by users | |
| </FormControl.Label> | |
| <ToggleSwitch | |
| size="small" | |
| checked={groupByUsers} | |
| onChange={() => setGroupByUsers(!groupByUsers)} | |
| /> | |
| </FormControl> |
Comment on lines
+72
to
+110
| /** Groups items by user login, preserving configured user order. */ | ||
| const groupItemsByUser = ( | ||
| items: GitHubItem[], | ||
| configuredUsernames: string[] | ||
| ): Record<string, GitHubItem[]> => { | ||
| const userGroups: Record<string, GitHubItem[]> = {}; | ||
| const configuredLower = configuredUsernames.map(u => u.toLowerCase()); | ||
|
|
||
| items.forEach(item => { | ||
| const login = item.user.login; | ||
| if (!userGroups[login]) { | ||
| userGroups[login] = []; | ||
| } | ||
| userGroups[login].push(item); | ||
| }); | ||
|
|
||
| // Order: configured users first (in order), then others alphabetically | ||
| const ordered: Record<string, GitHubItem[]> = {}; | ||
| for (const configUser of configuredUsernames) { | ||
| // Find the actual case-sensitive key that matches | ||
| const matchKey = Object.keys(userGroups).find( | ||
| k => k.toLowerCase() === configUser.toLowerCase() | ||
| ); | ||
| if (matchKey && userGroups[matchKey].length > 0) { | ||
| ordered[matchKey] = userGroups[matchKey]; | ||
| } | ||
| } | ||
| // Add remaining users not in configured list | ||
| Object.keys(userGroups) | ||
| .filter(k => !configuredLower.includes(k.toLowerCase())) | ||
| .sort((a, b) => a.toLowerCase().localeCompare(b.toLowerCase())) | ||
| .forEach(k => { | ||
| if (userGroups[k].length > 0) { | ||
| ordered[k] = userGroups[k]; | ||
| } | ||
| }); | ||
|
|
||
| return ordered; | ||
| }; |
There was a problem hiding this comment.
The groupItemsByUser function is duplicated identically across three view files (Summary.tsx lines 72-110, IssuesAndPRsList.tsx lines 113-148, EventView.tsx lines 41-76). This violates the DRY principle and creates maintenance burden. Consider extracting this function to a shared utility file (e.g., src/utils/viewFiltering.ts or a new src/utils/userGrouping.ts) to ensure consistent behavior and easier updates across all views.
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
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.
When multiple users are entered (comma-separated), all three views
(Summary, Issues & PRs, Events) now group their content by user with
collapsible user sections showing avatar, username, and item count.
Deduplication is skipped in multi-user mode to preserve per-user items
that may share the same URL across different users' activity.
Single-user mode behavior is completely unchanged.
https://claude.ai/code/session_01CKTzchrxuW2rxEjK9Aq5FK
Summary by CodeRabbit