Conversation
Contributor
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... 📒 Files selected for processing (2)
✏️ Tip: You can disable in-progress messages and the fortune message in your review settings. 📝 WalkthroughWalkthroughThis PR extends user profile interactivity by making avatars and usernames clickable across components to open profile dialogs, introduces a reusable Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Component as Avatar Component
participant Composable as useUserProfileDialog
participant Dialog as UserProfileDialog
participant Card as UserProfileCard
participant API as Server APIs
participant GitHub as GitHub API
User->>Component: Click avatar
Component->>Composable: openProfile(login)
Composable->>Dialog: open(login)
Dialog->>Dialog: Show modal
Dialog->>Card: Pass login prop
Card->>API: GET /api/user/profile?login
Card->>API: GET /api/user/profile-readme?login
Card->>API: GET /api/user/shared-repos?login
Card->>API: GET /api/user/activity?login
par Parallel API Calls
API->>GitHub: Fetch profile (REST + GraphQL)
API->>GitHub: Fetch README
API->>GitHub: Fetch shared repos
API->>GitHub: Fetch activity events
end
API-->>Card: Return aggregated data
Card->>Card: Render profile, README, repos, activity
Card-->>Dialog: Display enriched profile
Dialog-->>User: Show profile modal
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
server/api/user/shared-repos.get.ts (1)
46-46: Normalize cache key casing to avoid duplicate entries.Line 46 should normalize login casing so
UserA:UserBandusera:userbdon’t fragment cache entries.Proposed fix
- getKey: (_token: string, myLogin: string, otherLogin: string) => `${myLogin}:${otherLogin}`, + getKey: (_token: string, myLogin: string, otherLogin: string) => + `${myLogin.toLowerCase()}:${otherLogin.toLowerCase()}`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/api/user/shared-repos.get.ts` at line 46, The cache key generator getKey currently uses raw login strings which allows casing variations to create duplicate cache entries; update getKey to normalize both inputs (e.g., convert myLogin and otherLogin to a consistent case and trim whitespace) before composing the key—use safe conversion (String(...) or defensive checks) then template the normalized values as `${normalizedMy}:${normalizedOther}` so "UserA:UserB" and "usera:userb" map to the same cache key.server/utils/github.ts (1)
50-66: Good centralization of login validation.The
getLoginQueryhelper consolidates login parameter validation, reducing duplication across API routes. TheGITHUB_LOGIN_PATTERNcorrectly enforces GitHub's username constraints: up to 39 characters, alphanumeric characters and single hyphens only, no leading/trailing/consecutive hyphens.Consider differentiating the error message between missing (
!login) and invalid (fails regex) cases for clearer client feedback.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/utils/github.ts` around lines 50 - 66, Update getLoginQuery (and similarly getOrgQuery if desired) to throw distinct error messages for missing vs invalid values: first trim and check for falsy (throw createError with statusCode 400 and message like "Missing login parameter"), then validate against GITHUB_LOGIN_PATTERN and if it fails throw createError with statusCode 400 and message "Invalid login parameter"; apply the same two-step check to getOrgQuery (use "Missing org parameter" vs "Invalid org parameter") so clients receive clearer feedback.server/api/user/profile.get.ts (1)
69-71: Minor: Redundant query parameter access.
getQuery(event).loginis checked, thengetLoginQuery(event)reads and validates it again. This works but could be slightly streamlined.♻️ Optional simplification
- if (getQuery(event).login) { - return fetchOtherProfile(token, userId, getLoginQuery(event)) + const loginParam = getQuery(event).login as string | undefined + if (loginParam) { + const login = getLoginQuery(event) // validates + return fetchOtherProfile(token, userId, login) }Alternatively, you could make
getLoginQueryreturnstring | undefinedwhen the param is absent, avoiding the double-read entirely.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/api/user/profile.get.ts` around lines 69 - 71, The code redundantly reads the login query twice; call getLoginQuery(event) once, store its result in a variable (e.g., login = getLoginQuery(event)), check that variable for truthiness, and if present pass it to fetchOtherProfile(token, userId, login); update getLoginQuery to return string | undefined if necessary so the single read covers both the existence check and the validated value used by fetchOtherProfile.server/api/user/profile-readme.get.ts (1)
13-13: Consider handling missing or truncated content.GitHub may omit the
contentfield for large files (>1MB) and instead provide adownload_url. Ifdata.contentis undefined,Buffer.from(undefined, 'base64')will throw.🛡️ Optional defensive check
- return Buffer.from(data.content, 'base64').toString('utf-8') + if (!data.content) return null + return Buffer.from(data.content, 'base64').toString('utf-8')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/api/user/profile-readme.get.ts` at line 13, The current return uses Buffer.from(data.content, 'base64').toString('utf-8') and will throw if data.content is undefined; update the handler that returns the README (the code around Buffer.from(data.content...)) to first check for data.content and if missing, fetch the raw text from data.download_url (or return a clear error) and then return the UTF-8 string; ensure you reference data.content and data.download_url and avoid calling Buffer.from with undefined.app/components/user/UserProfileCard.vue (1)
345-345: Consider a dedicated i18n key instead of reusingissues.sidebar.showLess.Reusing a translation key from the issues domain creates implicit coupling. If the issues translation changes context (e.g., "Show fewer issues"), this component's UI could become confusing.
💡 Suggested approach
Add a generic or profile-specific key:
- {{ activityExpanded ? t('issues.sidebar.showLess') : `+${activity.length - 3}` }} + {{ activityExpanded ? t('common.showLess') : `+${activity.length - 3}` }}Then add the key to your i18n files under a
commonnamespace.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/user/UserProfileCard.vue` at line 345, Replace the hardcoded reuse of the issues domain key in UserProfileCard.vue (the expression using activityExpanded, activity.length and t('issues.sidebar.showLess')) with a dedicated i18n key (e.g., t('common.activity.showMore') or t('profile.activity.showMore')) and update the toggle text logic to use that key when activityExpanded is false; then add the new key(s) to the i18n resource files (under a common or profile namespace) with appropriate translations for both states so the component no longer depends on issues.sidebar.showLess.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/components/user/UserProfileCard.vue`:
- Around line 72-82: The switch in activityLabel(ev: UserActivityEvent) is
missing a default/exhaustiveness branch so it can return undefined for new
activity types; add a final default case that both returns a safe fallback label
(e.g. t('user.profile.activity.unknown') or similar) and enforces compile-time
exhaustiveness by assigning ev to a never-typed variable (e.g. default: const
_exhaustive: never = ev; return t(...)), ensuring TypeScript will error if
UserActivityEvent gains new variants and preventing undefined being rendered.
In `@server/api/user/shared-repos.get.ts`:
- Around line 20-24: The two githubFetchAllWithToken calls that fetch repos with
params { per_page: 100, type: 'owner' } will never yield matching full_name
values for shared repos; update the params for both calls (the ones invoking
githubFetchAllWithToken<GitHubRepo>(token, `/users/${otherLogin}/repos`, ...)
and the counterpart for the current user) to use type: 'all' instead of 'owner'
so collaborator repositories are included and shared full_name matches are
possible.
In `@server/utils/profile.ts`:
- Line 26: The mapper currently forwards user.pronouns directly which can
produce undefined; update the mapper that sets pronouns (the line using
user.pronouns) to normalize undefined to null (e.g. use the nullish-coalescing
check on user.pronouns so the output property is either a string or null),
ensuring the mapped pronouns field never leaks undefined.
---
Nitpick comments:
In `@app/components/user/UserProfileCard.vue`:
- Line 345: Replace the hardcoded reuse of the issues domain key in
UserProfileCard.vue (the expression using activityExpanded, activity.length and
t('issues.sidebar.showLess')) with a dedicated i18n key (e.g.,
t('common.activity.showMore') or t('profile.activity.showMore')) and update the
toggle text logic to use that key when activityExpanded is false; then add the
new key(s) to the i18n resource files (under a common or profile namespace) with
appropriate translations for both states so the component no longer depends on
issues.sidebar.showLess.
In `@server/api/user/profile-readme.get.ts`:
- Line 13: The current return uses Buffer.from(data.content,
'base64').toString('utf-8') and will throw if data.content is undefined; update
the handler that returns the README (the code around
Buffer.from(data.content...)) to first check for data.content and if missing,
fetch the raw text from data.download_url (or return a clear error) and then
return the UTF-8 string; ensure you reference data.content and data.download_url
and avoid calling Buffer.from with undefined.
In `@server/api/user/profile.get.ts`:
- Around line 69-71: The code redundantly reads the login query twice; call
getLoginQuery(event) once, store its result in a variable (e.g., login =
getLoginQuery(event)), check that variable for truthiness, and if present pass
it to fetchOtherProfile(token, userId, login); update getLoginQuery to return
string | undefined if necessary so the single read covers both the existence
check and the validated value used by fetchOtherProfile.
In `@server/api/user/shared-repos.get.ts`:
- Line 46: The cache key generator getKey currently uses raw login strings which
allows casing variations to create duplicate cache entries; update getKey to
normalize both inputs (e.g., convert myLogin and otherLogin to a consistent case
and trim whitespace) before composing the key—use safe conversion (String(...)
or defensive checks) then template the normalized values as
`${normalizedMy}:${normalizedOther}` so "UserA:UserB" and "usera:userb" map to
the same cache key.
In `@server/utils/github.ts`:
- Around line 50-66: Update getLoginQuery (and similarly getOrgQuery if desired)
to throw distinct error messages for missing vs invalid values: first trim and
check for falsy (throw createError with statusCode 400 and message like "Missing
login parameter"), then validate against GITHUB_LOGIN_PATTERN and if it fails
throw createError with statusCode 400 and message "Invalid login parameter";
apply the same two-step check to getOrgQuery (use "Missing org parameter" vs
"Invalid org parameter") so clients receive clearer feedback.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
app/components/focus/CreatedIssueCard.vueapp/components/focus/InboxUnifiedCard.vueapp/components/issue/IssueHeader.vueapp/components/issue/IssueRow.vueapp/components/issue/IssueSidebar.vueapp/components/repo/RepoStatistics.vueapp/components/user/UserCard.vueapp/components/user/UserChip.vueapp/components/user/UserProfileCard.vueapp/components/user/UserProfileDialog.vueapp/composables/useUserProfileDialog.tsi18n/locales/de.jsoni18n/locales/en.jsoni18n/schema.jsonserver/api/user/activity.get.tsserver/api/user/profile-readme.get.tsserver/api/user/profile.get.tsserver/api/user/shared-repos.get.tsserver/utils/activity.tsserver/utils/github.tsserver/utils/profile.tsshared/types/profile.tstest/nuxt/profileStore.test.tstest/nuxt/userProfileDialog.test.tstest/unit/activity.test.ts
Comment on lines
+72
to
+82
| function activityLabel(ev: UserActivityEvent) { | ||
| switch (ev.type) { | ||
| case 'push': return t('user.profile.activity.push', { branch: ev.ref ?? 'main' }) | ||
| case 'pr': return t('user.profile.activity.pr', { action: ev.action ?? 'opened', number: ev.number ?? 0 }) | ||
| case 'issue': return t('user.profile.activity.issue', { action: ev.action ?? 'opened', number: ev.number ?? 0 }) | ||
| case 'create': return t('user.profile.activity.create', { type: ev.refType ?? 'branch', ref: ev.ref ?? '' }) | ||
| case 'release': return t('user.profile.activity.release', { tag: ev.tagName ?? '' }) | ||
| case 'star': return t('user.profile.activity.starred') | ||
| case 'fork': return t('user.profile.activity.forked') | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Add a default case or exhaustiveness check to activityLabel.
The switch statement lacks a default case. If ActivityType is extended in the future, this function will silently return undefined, rendering as empty text in the template.
🛡️ Proposed fix with exhaustiveness check
function activityLabel(ev: UserActivityEvent) {
switch (ev.type) {
case 'push': return t('user.profile.activity.push', { branch: ev.ref ?? 'main' })
case 'pr': return t('user.profile.activity.pr', { action: ev.action ?? 'opened', number: ev.number ?? 0 })
case 'issue': return t('user.profile.activity.issue', { action: ev.action ?? 'opened', number: ev.number ?? 0 })
case 'create': return t('user.profile.activity.create', { type: ev.refType ?? 'branch', ref: ev.ref ?? '' })
case 'release': return t('user.profile.activity.release', { tag: ev.tagName ?? '' })
case 'star': return t('user.profile.activity.starred')
case 'fork': return t('user.profile.activity.forked')
+ default: {
+ const _exhaustive: never = ev.type
+ return ''
+ }
}
}📝 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.
Suggested change
| function activityLabel(ev: UserActivityEvent) { | |
| switch (ev.type) { | |
| case 'push': return t('user.profile.activity.push', { branch: ev.ref ?? 'main' }) | |
| case 'pr': return t('user.profile.activity.pr', { action: ev.action ?? 'opened', number: ev.number ?? 0 }) | |
| case 'issue': return t('user.profile.activity.issue', { action: ev.action ?? 'opened', number: ev.number ?? 0 }) | |
| case 'create': return t('user.profile.activity.create', { type: ev.refType ?? 'branch', ref: ev.ref ?? '' }) | |
| case 'release': return t('user.profile.activity.release', { tag: ev.tagName ?? '' }) | |
| case 'star': return t('user.profile.activity.starred') | |
| case 'fork': return t('user.profile.activity.forked') | |
| } | |
| } | |
| function activityLabel(ev: UserActivityEvent) { | |
| switch (ev.type) { | |
| case 'push': return t('user.profile.activity.push', { branch: ev.ref ?? 'main' }) | |
| case 'pr': return t('user.profile.activity.pr', { action: ev.action ?? 'opened', number: ev.number ?? 0 }) | |
| case 'issue': return t('user.profile.activity.issue', { action: ev.action ?? 'opened', number: ev.number ?? 0 }) | |
| case 'create': return t('user.profile.activity.create', { type: ev.refType ?? 'branch', ref: ev.ref ?? '' }) | |
| case 'release': return t('user.profile.activity.release', { tag: ev.tagName ?? '' }) | |
| case 'star': return t('user.profile.activity.starred') | |
| case 'fork': return t('user.profile.activity.forked') | |
| default: { | |
| const _exhaustive: never = ev.type | |
| return '' | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/user/UserProfileCard.vue` around lines 72 - 82, The switch in
activityLabel(ev: UserActivityEvent) is missing a default/exhaustiveness branch
so it can return undefined for new activity types; add a final default case that
both returns a safe fallback label (e.g. t('user.profile.activity.unknown') or
similar) and enforces compile-time exhaustiveness by assigning ev to a
never-typed variable (e.g. default: const _exhaustive: never = ev; return
t(...)), ensuring TypeScript will error if UserActivityEvent gains new variants
and preventing undefined being rendered.
| login: user.login, | ||
| name: user.name, | ||
| avatarUrl: user.avatar_url, | ||
| pronouns: user.pronouns, |
Contributor
There was a problem hiding this comment.
Normalize pronouns to null in the mapper output.
Line 26 currently forwards the raw value directly; normalizing avoids leaking undefined into a string | null contract.
Proposed fix
- pronouns: user.pronouns,
+ pronouns: user.pronouns ?? null,📝 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.
Suggested change
| pronouns: user.pronouns, | |
| pronouns: user.pronouns ?? null, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/utils/profile.ts` at line 26, The mapper currently forwards
user.pronouns directly which can produce undefined; update the mapper that sets
pronouns (the line using user.pronouns) to normalize undefined to null (e.g. use
the nullish-coalescing check on user.pronouns so the output property is either a
string or null), ensuring the mapped pronouns field never leaks undefined.
This was referenced Mar 7, 2026
Merged
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.
Summary
Related issue(s)
Closes #146
Type of change
Checklist
Summary by CodeRabbit
New Features
UI/UX Improvements
Localization