Skip to content

feat: show current author first in PR filter#2139

Merged
arnestrickmann merged 3 commits into
generalaction:mainfrom
mezotv:emdash/show-current-author-first-2z4h5
May 20, 2026
Merged

feat: show current author first in PR filter#2139
arnestrickmann merged 3 commits into
generalaction:mainfrom
mezotv:emdash/show-current-author-first-2z4h5

Conversation

@mezotv
Copy link
Copy Markdown
Contributor

@mezotv mezotv commented May 20, 2026

Mimics GitHub's behavior for PRs by showing the current GitHub user first in the author filter.

@mezotv mezotv marked this pull request as ready for review May 20, 2026 07:45
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 20, 2026

Greptile Summary

This PR surfaces the authenticated GitHub user at the top of the PR author filter dropdown, mirroring GitHub's own UI behaviour. It extracts a small pr-filter-items.ts module with a reusable usersWithLoginFirst helper and toUserItem mapper, wires in useGithubContext to obtain the current login, and ships a test file for the new helper.

  • pr-filter-items.ts: New module exporting UserItem, toUserItem, and usersWithLoginFirst; the reorder logic is correct and gracefully handles unauthenticated state (null/undefined login) and unknown users.
  • usePrViewState.ts: Consumes useGithubContext().user?.login and delegates to the new helper; user?.login is correctly added to the useMemo dependency array.
  • pr-view.tsx: Import of UserItem moved to the canonical pr-filter-items module; no functional change.

Confidence Score: 4/5

Safe to merge; the change is additive and well-scoped, with a graceful no-op fallback when the current user is unauthenticated or absent from the author list.

The reordering logic and hook integration are correct. The only concerns are a case-sensitive string comparison that could silently miss the current user if any layer normalises GitHub logins differently, and a few straightforward test cases (null login, user already first) that would fully close out the helper's code paths.

pr-filter-items.ts (case-sensitive comparison on line 19) and pr-filter-items.test.ts (missing edge-case coverage).

Important Files Changed

Filename Overview
src/renderer/features/projects/components/pr-view/pr-filter-items.ts New file extracting UserItem type, toUserItem mapper, and usersWithLoginFirst reorder helper. Logic is correct for all exercised cases; case-sensitive username comparison is a minor robustness concern.
src/renderer/features/projects/components/pr-view/pr-filter-items.test.ts New test file covering the main happy path and user-not-found path; missing coverage for null/undefined login and user-already-first edge cases.
src/renderer/features/projects/components/pr-view/usePrViewState.ts Integrates useGithubContext to surface the current user's login, delegates ordering to usersWithLoginFirst, and correctly includes user?.login in the useMemo dependency array. Refactor of assigneeItems to toUserItem is clean.
src/renderer/features/projects/components/pr-view/pr-view.tsx Moves UserItem import to the canonical pr-filter-items module; no functional change.

Sequence Diagram

sequenceDiagram
    participant GHP as GithubContextProvider
    participant Hook as usePrViewState
    participant Helper as usersWithLoginFirst
    participant UI as PR Filter Dropdown

    GHP->>Hook: user.login (current GitHub login)
    Hook->>Hook: useFilterOptions → filterOptions.authors[]
    Hook->>Helper: usersWithLoginFirst(authors, user.login)
    Helper->>Helper: "findIndex where userName === login"
    alt current user found
        Helper-->>Hook: [currentUser, ...rest]
    else user not found / no login
        Helper-->>Hook: [...authors] (original order)
    end
    Hook->>Hook: .map(toUserItem) → authorItems[]
    Hook-->>UI: authorItems (current user first)
Loading
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
src/renderer/features/projects/components/pr-view/pr-filter-items.ts:19
**Case-sensitive login comparison may silently miss the current user**

The comparison `user.userName === login` is case-sensitive. GitHub's API returns logins with a canonical casing, and in practice both `GitHubUser.login` and `PullRequestUser.userName` come from GitHub, so they should match. However, if any layer normalises one to lowercase (database column, IPC serialisation, etc.) but not the other, the current user won't be promoted and no error will surface. A `.toLowerCase()` guard on both sides would make this resilient to any future discrepancy.

### Issue 2 of 2
src/renderer/features/projects/components/pr-view/pr-filter-items.test.ts:17-35
**Missing test cases for null/undefined login and current user already first**

The two existing tests cover the happy path and "user not found," but there are no cases for: (1) `login` being `null` or `undefined` (unauthenticated state — the function's falsy-guard branch), and (2) the current user already being the first element in the list. Both edge cases directly exercise distinct code paths in `usersWithLoginFirst` and are straightforward to add.

Reviews (1): Last reviewed commit: "Show current GitHub user first in PR aut..." | Re-trigger Greptile

Comment thread src/renderer/features/projects/components/pr-view/pr-filter-items.ts Outdated
@mezotv mezotv changed the title Show current author first in PR filter feat: show current author first in PR filter May 20, 2026
@arnestrickmann arnestrickmann merged commit ead3a57 into generalaction:main May 20, 2026
1 check passed
@mezotv mezotv deleted the emdash/show-current-author-first-2z4h5 branch May 20, 2026 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants