feature/friend requests #13
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 WalkthroughWalkthroughAdds a full friend request system: client-side provider/service and commands in the VS Code extension, shared types/validators, server-side Prisma schema and REST routes with WebSocket events, and a pre-commit type check step. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant VSCode as VS Code Extension
participant Service as FriendRequestService
participant Server
participant DB as Database
participant WS as WebSocket
User->>VSCode: Click "Add Friend"
VSCode->>Service: searchUsers(query)
Service->>Server: GET /api/users/search
Server->>DB: Query users
DB-->>Server: Users
Server-->>Service: Users[]
Service-->>VSCode: Display results
User->>VSCode: Select user
VSCode->>Service: sendRequest(toUserId)
Service->>Server: POST /api/v1/friend-requests
Server->>DB: Create FriendRequest (PENDING) / or detect reciprocal -> ACCEPTED
alt New request
Server->>WS: Broadcast FRIEND_REQUEST_RECEIVED to recipient
else Reciprocal exists
Server->>DB: Create bidirectional follow records
Server->>WS: Broadcast FRIEND_REQUEST_ACCEPTED to both users
end
Server-->>Service: FriendRequestDTO/Success
Service-->>VSCode: Emit onRequestsChanged / notify
VSCode->>VSCode: Refresh friend requests & friends view
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (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)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/server/src/routes/friends.ts (1)
238-281: Semantic inconsistency: DELETE endpoint removes bidirectional relationships, but POST creates only unidirectional ones.The DELETE /friends/:id endpoint removes relationships in both directions (lines 267-275), but POST /friends/:id creates only unidirectional follows (lines 220-222). This creates an asymmetry:
- POST /friends/:id: Creates A → B only
- DELETE /friends/:id: Removes A → B AND B → A
If user A follows user B, and user B separately follows A, then A's DELETE request will remove B's follow relationship without B's consent. This violates user autonomy and could cause unintended data loss.
The DELETE endpoint's comment "removes bidirectional follow relationship" and log "Users unfriended (bidirectional)" also suggest a misalignment with the actual unidirectional semantics of the POST endpoint.
Recommendation: Clarify the intended model and choose one approach:
- Separate semantics: Keep
/friends/:idfor unidirectional follows; use a separate endpoint/model for bidirectional friendships (friend requests)- Make POST bidirectional: Update POST to create relationships in both directions
- Make DELETE unidirectional: Only remove the specific direction created by the current user (A removes A → B only)
🤖 Fix all issues with AI agents
In @apps/extension/package.json:
- Around line 50-55: The view with id "devradar.friendRequests" references an
icon at "media/requests.svg" but that SVG is missing; fix by adding a new
requests.svg into the extension's media folder (matching the style/size of
existing icons like activity.svg/friends.svg) or update the view's "icon"
property to point to an existing asset; ensure the filename exactly matches
"media/requests.svg" and the SVG is valid and exported so the Friend Requests
view can load its icon.
In @apps/extension/src/extension.ts:
- Around line 137-168: The command 'devradar.focusFriendRequests' is registered
in extension.ts but not declared in apps/extension/package.json, so it won't
appear in the command palette; either add an entry for
'devradar.focusFriendRequests' under the package.json "contributes.commands"
array (provide an id, title and optional category) to make it user-accessible,
or if it is meant to be internal-only, add a clear comment next to the handler
in extension.ts documenting that intent and leave it out of package.json.
- Around line 615-655: handleUnfriend currently performs a raw fetch and
duplicates auth/HTTP logic and has a redundant response check; add an
unfriend(userId: string) method to FriendRequestService (or the appropriate
service) that handles building the URL, attaching the token (using
authService.getToken), performing the DELETE and throwing on non-ok responses,
then replace the inline fetch in handleUnfriend with a call to
FriendRequestService.unfriend(item.userId); keep the existing try/catch and
friendsProvider.refresh() flow, and if you opt to keep an inline fetch instead,
simplify the check to if (!response.ok) (remove the redundant response.status
!== 204 clause).
In @apps/extension/src/services/friendRequestService.ts:
- Around line 66-88: searchUsers (and other fetch callers in
friendRequestService.ts) can hang because there is no request timeout; add a
private helper like fetchWithTimeout that uses AbortController to abort after a
default timeout (e.g., 10_000 ms), ensure you pass the controller.signal into
fetch, clear the timeout in a finally block, and replace the direct fetch call
in searchUsers with this.fetchWithTimeout using the same headers and URL; apply
the same helper to all other fetch calls in this service so every network
request is bounded by the timeout.
- Around line 232-235: The condition checking `response.status !== 204` is
redundant because a 204 is already treated as success (`response.ok` true);
change the conditional from `if (!response.ok && response.status !== 204)` to
simply `if (!response.ok)` in the error-handling block (the code that reads
`const errorData = (await response.json()) as { error?: { message?: string } };
throw new Error(...)`) so the same error parsing/throwing logic executes only
when the response is not OK.
In @apps/server/prisma/schema.prisma:
- Around line 48-65: The FriendRequest model's @@unique([fromId, toId])
constraint prevents resending after a REJECTED request; remove or adjust that
uniqueness and implement cleanup or status-aware checks: either delete
REJECTED/EXPIRED rows before inserting a new FriendRequest (in the code path
that creates requests) or remove the @@unique([fromId, toId]) constraint and
enforce uniqueness in application logic by querying for existing PENDING
requests only; as an alternative you can change the schema to @@unique([fromId,
toId, status]) if you want uniqueness per status.
In @apps/server/src/routes/friendRequests.ts:
- Around line 132-144: The current check using db.follow.findFirst
(existingFollow) incorrectly treats a single-direction follow as "already
friends"; update the logic to assert mutual follows exist by checking for BOTH
follow records (followerId: userId → followingId: toUserId AND followerId:
toUserId → followingId: userId). Replace the findFirst with a query that
verifies both records (e.g., findMany with those two where clauses and ensure
count === 2, or perform two findUnique/findFirst calls) and only throw
ConflictError('You are already friends with this user') when both follow rows
are present.
- Around line 53-68: The file defines duplicate interfaces PublicUserDTO and
FriendRequestDTO; remove these local duplicates and import the canonical types
from the shared package (the types exported from packages/shared/src/types.ts),
then update the top-of-file imports to reference PublicUserDTO and
FriendRequestDTO instead of the local declarations and ensure any uses (e.g., in
route handlers or function signatures) keep the same names but rely on the
imported types.
- Around line 22-25: Remove the local SendFriendRequestSchema duplicate and
import the shared schema from the validators module instead; replace the local
const SendFriendRequestSchema declaration with an import of the existing
SendFriendRequestSchema from the shared validators (the same module that
provides PaginationQuerySchema) and update any references to use that imported
symbol.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (13)
.husky/pre-commitapps/extension/package.jsonapps/extension/src/extension.tsapps/extension/src/services/friendRequestService.tsapps/extension/src/services/index.tsapps/extension/src/views/friendRequestsProvider.tsapps/extension/src/views/index.tsapps/server/prisma/schema.prismaapps/server/src/routes/friendRequests.tsapps/server/src/routes/friends.tsapps/server/src/server.tspackages/shared/src/types.tspackages/shared/src/validators.ts
🧰 Additional context used
🧬 Code graph analysis (6)
apps/server/src/server.ts (1)
apps/server/src/routes/friendRequests.ts (1)
friendRequestRoutes(97-448)
apps/server/src/routes/friends.ts (2)
apps/server/src/lib/errors.ts (1)
NotFoundError(121-129)apps/server/src/lib/logger.ts (1)
logger(72-72)
apps/extension/src/extension.ts (2)
apps/extension/src/views/friendRequestsProvider.ts (1)
FriendRequestsProvider(76-179)packages/shared/src/types.ts (2)
FriendRequestDTO(101-108)PublicUserDTO(93-98)
apps/server/src/routes/friendRequests.ts (6)
packages/shared/src/validators.ts (2)
SendFriendRequestSchema(81-83)PaginationQuerySchema(75-78)packages/shared/src/types.ts (2)
PublicUserDTO(93-98)FriendRequestDTO(101-108)apps/server/src/services/db.ts (1)
getDb(20-33)apps/server/src/lib/errors.ts (4)
ValidationError(67-81)ConflictError(134-145)NotFoundError(121-129)AuthorizationError(108-116)apps/server/src/lib/logger.ts (1)
logger(72-72)apps/server/src/ws/handler.ts (1)
broadcastToUsers(423-430)
apps/extension/src/services/friendRequestService.ts (2)
packages/shared/src/types.ts (3)
FriendRequestDTO(101-108)PublicUserDTO(93-98)PaginatedResponse(144-152)apps/extension/src/utils/logger.ts (1)
error(51-59)
apps/extension/src/views/friendRequestsProvider.ts (4)
packages/shared/src/types.ts (1)
FriendRequestDTO(101-108)apps/extension/src/views/index.ts (1)
FriendRequestsProvider(2-2)apps/server/src/lib/logger.ts (1)
logger(72-72)apps/extension/src/utils/logger.ts (1)
error(51-59)
🔇 Additional comments (37)
.husky/pre-commit (1)
1-1: LGTM! Good addition of type checking.Adding type checking before lint-staged is a best practice that helps catch type errors early in the development workflow.
packages/shared/src/types.ts (1)
10-11: LGTM! Well-structured type definitions.The new types for friend request functionality are well-defined:
FriendRequestStatusprovides clear status valuesPublicUserDTOappropriately limits exposed user informationFriendRequestDTOand payload interfaces are properly structured for the friend request workflow- New
MessageTypevalues support real-time notificationsAlso applies to: 17-17, 25-27, 92-119
packages/shared/src/validators.ts (1)
12-13: LGTM! Validators align with type definitions.The new validation schemas properly support the friend request functionality:
FriendRequestStatusSchemamatches theFriendRequestStatustypeMessageTypeSchemaincludes all new WebSocket message typesSendFriendRequestSchemavalidates the friend request payload with appropriate constraintsAlso applies to: 25-27, 80-94
apps/server/prisma/schema.prisma (2)
28-29: LGTM! User model relations properly configured.The new
sentFriendRequestsandreceivedFriendRequestsrelations are correctly defined and will support bidirectional friend request queries.
84-96: LGTM! Good schema improvements.The changes improve data integrity and query performance:
TeamMembernow has a unique constraint on[userId, teamId]preventing duplicate memberships- Indexes on
userIdandteamIdwill improve query performanceFriendRequestStatusenum is well-definedAlso applies to: 110-114
apps/server/src/routes/friendRequests.ts (6)
212-292: LGTM!The incoming and outgoing request endpoints correctly implement pagination with sensible defaults, include related user data, and calculate
hasMoreaccurately.
294-338: LGTM!The accept endpoint properly validates the request ID, verifies ownership (only the recipient can accept), ensures the request is still pending, and delegates to the internal helper for atomic processing.
340-385: LGTM!The reject endpoint correctly validates authorization and status before updating. Keeping rejected requests (rather than deleting) allows for audit trails.
387-429: LGTM!The cancel endpoint correctly restricts the action to the sender and properly deletes the request with a 204 response.
431-448: LGTM!Simple and efficient count endpoint for badge display purposes.
450-494: LGTM!The internal helper properly uses a transaction for atomicity, handles potential duplicate follows gracefully with
skipDuplicates, and broadcasts acceptance events to both parties after the transaction commits.apps/server/src/server.ts (2)
25-25: LGTM!The import is correctly placed and follows the existing naming convention.
217-217: LGTM!The route registration follows the established pattern and the
/friend-requestsprefix appropriately aligns with the route module's endpoint design.apps/extension/package.json (2)
114-149: LGTM!The new commands are well-structured with appropriate icons and categorization. The naming is consistent with existing commands.
157-197: LGTM!Menu configurations correctly use
whenclauses to scope actions:
- Accept/reject only shown for incoming requests (
friendRequest-incoming)- Cancel only shown for outgoing requests (
friendRequest-outgoing)- Unfriend scoped to friend items with
viewItem =~ /^friend-/- Inline grouping with
@1and@2ensures consistent action orderingapps/extension/src/services/index.ts (1)
4-4: LGTM!The barrel export follows the established pattern for exposing services.
apps/extension/src/views/index.ts (1)
2-2: LGTM!The barrel export follows the established pattern for exposing view providers.
apps/extension/src/views/friendRequestsProvider.ts (6)
12-51: LGTM!The
FriendRequestTreeItemclass properly displays user info based on direction, uses appropriate theme icons with colors, and builds informative tooltips with Markdown.
53-71: LGTM!The
SectionTreeItemclass provides clear section headers with counts and appropriate initial expansion state (expanded if items exist).
75-101: LGTM!The provider constructor properly initializes resources and subscribes to the service's change events for automatic refresh. Disposables are correctly tracked.
103-130: LGTM!The tree data methods correctly implement the two-level hierarchy: sections at root level and request items as children.
132-160: LGTM!The refresh method properly:
- Prevents concurrent refreshes with
isLoadingguard- Fetches data in parallel with
Promise.all- Preserves existing data on error
- Always fires the change event in
finally
162-179: LGTM!The utility methods (
clear,getPendingCount,dispose) are straightforward and correctly implemented.apps/extension/src/extension.ts (8)
10-11: LGTM!The imports for
FriendRequestServiceandFriendRequestsProviderare correctly added.
42-66: LGTM!The service and provider initialization follows the established pattern, and disposables are correctly tracked for cleanup.
94-99: LGTM!The friend requests tree view is correctly registered alongside existing views.
224-235: LGTM!WebSocket message handling for friend request events is correctly implemented, with appropriate type casting and triggering a friends list refresh on acceptance.
260-262: LGTM!Conditionally refreshing friend requests only when authenticated prevents unnecessary network calls.
480-540: LGTM!The
handleAddFriendflow is well-structured with proper input validation, user search, filtering, and error handling.
542-555: LGTM!The
isFriendRequestItemtype guard thoroughly validates the nested structure.
557-608: LGTM!The accept, reject, and cancel handlers follow a consistent pattern with proper validation, service calls, user feedback, and error handling. The accept handler correctly refreshes the friends list.
apps/extension/src/services/friendRequestService.ts (6)
1-22: LGTM!Clean imports and well-defined event interface that properly types the three event streams exposed by the service.
24-49: LGTM!Good dependency injection and proper disposal pattern. EventEmitters are correctly tracked in the disposables array for cleanup.
51-63: LGTM!Authentication enforcement is properly implemented by throwing when no token is available.
117-157: LGTM!Pagination is correctly implemented with sensible defaults. The same timeout considerations flagged earlier apply here.
159-176: LGTM!Returning
0on error is appropriate for a badge count - graceful degradation without disrupting the UI.
263-278: LGTM!Event handler and disposal logic are correctly implemented.
…nd method - Add fetchWithTimeout helper with 10s default timeout using AbortController - Add unfriend() method to FriendRequestService - Improve error handling for JSON parsing in error responses - Fix notification error handling with proper catch and user feedback
- Import SendFriendRequestSchema from @devradar/shared instead of local duplicate - Import PublicUserDTO and FriendRequestDTO types from @devradar/shared - Fix 'already friends' check to verify mutual follows (both directions) - Add cleanup of REJECTED requests before creating new ones
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @apps/server/src/routes/friendRequests.ts:
- Around line 162-170: The existence check for the authenticated user (the
db.user.findUnique call that assigns currentUser and the subsequent
NotFoundError throw) must be moved earlier in the handler to run before any
state-changing operations (e.g., before deleting rejected requests) so we never
mutate data for a user that no longer exists; locate the currentUser variable
and NotFoundError usage in the friend requests route/handler and move that check
to run immediately after authentication (or alternatively remove the redundant
check if you can guarantee authentication implies existence), ensuring all
references to currentUser still point to the validated result.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/extension/media/requests.svgis excluded by!**/*.svg
📒 Files selected for processing (5)
apps/extension/CHANGELOG.mdapps/extension/package.jsonapps/extension/src/extension.tsapps/extension/src/services/friendRequestService.tsapps/server/src/routes/friendRequests.ts
🧰 Additional context used
🧬 Code graph analysis (2)
apps/server/src/routes/friendRequests.ts (6)
packages/shared/src/types.ts (2)
PublicUserDTO(93-98)FriendRequestDTO(101-108)apps/server/src/services/db.ts (1)
getDb(20-33)packages/shared/src/validators.ts (2)
SendFriendRequestSchema(81-83)PaginationQuerySchema(75-78)apps/server/src/lib/errors.ts (4)
ValidationError(67-81)ConflictError(134-145)NotFoundError(121-129)AuthorizationError(108-116)apps/server/src/lib/logger.ts (1)
logger(72-72)apps/server/src/ws/handler.ts (1)
broadcastToUsers(423-430)
apps/extension/src/services/friendRequestService.ts (3)
packages/shared/src/types.ts (3)
FriendRequestDTO(101-108)PublicUserDTO(93-98)PaginatedResponse(144-152)apps/extension/src/services/index.ts (1)
FriendRequestService(4-4)apps/extension/src/utils/logger.ts (1)
error(51-59)
🔇 Additional comments (24)
apps/extension/CHANGELOG.md (1)
8-24: LGTM!The changelog entry is well-structured, follows the Keep a Changelog format, and comprehensively documents the new friend request system features.
apps/server/src/routes/friendRequests.ts (5)
1-27: LGTM!The imports, schema definitions, and type interfaces are well-organized. The
RequestIdParamsSchemaprovides proper validation for route parameters.
49-73: LGTM!The DTO conversion functions are clean and correctly transform database entities to API response shapes.
200-280: LGTM!The incoming and outgoing request endpoints are well-implemented with proper pagination, parallel database queries, and consistent response structure.
282-417: LGTM!The accept, reject, and cancel routes have proper authorization checks, status validation, and appropriate error handling. The distinction between recipient-only operations (accept/reject) and sender-only operations (cancel) is correctly enforced.
438-482: LGTM!The
acceptRequestInternalfunction correctly uses a transaction for atomicity, creates bidirectional follow entries withskipDuplicatesfor safety, and broadcasts acceptance events to both users with appropriate friend information.apps/extension/src/extension.ts (6)
10-11: LGTM!The new service and provider are properly imported, declared, initialized, and added to disposables for proper lifecycle management.
Also applies to: 30-31, 42-52
94-97: LGTM!The tree view registration and new command handlers are correctly wired up. The command structure follows the existing pattern.
Also applies to: 137-168
224-235: LGTM!The WebSocket message handlers correctly process friend request events, delegate to the service for notifications, and refresh the friends list when friendships are established.
480-540: LGTM!The
handleAddFriendmethod has good UX flow with validation, filtering out the current user, and proper error handling with user-friendly messages.
542-555: LGTM!The
isFriendRequestItemtype guard is thorough with proper null checks and property existence verification.
557-642: LGTM!The friend request action handlers (accept, reject, cancel, unfriend) follow consistent patterns with proper type checking, error handling, and appropriate UI feedback. The unfriend action includes a confirmation dialog which is good UX for a destructive operation.
apps/extension/package.json (4)
5-5: LGTM!Version bump to 0.2.0 aligns with the new feature release documented in the changelog.
114-154: LGTM!The new commands are well-defined with appropriate titles, categories, and icons using VS Code's built-in product icon set.
162-202: LGTM!The menu contributions are correctly configured with appropriate
whenclauses to ensure actions appear in the right context. The inline grouping for accept/reject buttons and the regex pattern for unfriend (viewItem =~ /^friend-/) are well thought out.
50-55: No action required—icon file exists.The
requests.svgfile is present atapps/extension/media/requests.svg.apps/extension/src/services/friendRequestService.ts (8)
17-49: LGTM!The event emitter setup is well-structured with proper typing and the emitters are correctly added to disposables for cleanup.
51-88: LGTM!The helper methods are well-implemented. The
fetchWithTimeoutcorrectly usesAbortControllerwith proper cleanup in thefinallyblock to prevent memory leaks.
90-114: LGTM!The
searchUsersmethod correctly validates input length, encodes the query parameter, and handles errors appropriately.
116-151: LGTM!The
sendRequestmethod has proper error extraction from server responses and fires the change event on success.
153-215: LGTM!The pagination methods (
getIncomingRequests,getOutgoingRequests) andgetPendingCountare correctly implemented. The silent fallback to 0 forgetPendingCountis appropriate for a badge display feature.
217-308: LGTM!The accept, reject, and cancel methods have consistent error handling patterns and properly account for 204 No Content responses on DELETE operations.
310-341: LGTM!The WebSocket event handlers provide excellent UX with actionable notifications. The inline accept action from the notification is a nice touch with proper error handling.
343-376: LGTM!The
unfriendmethod correctly uses the/friends/{userId}endpoint and handles the 204 response appropriately. The dispose method properly cleans up all event emitters.
| /* Get current user info for the response */ | ||
| const currentUser = await db.user.findUnique({ | ||
| where: { id: userId }, | ||
| select: { id: true, username: true, displayName: true, avatarUrl: true }, | ||
| }); | ||
|
|
||
| if (!currentUser) { | ||
| throw new NotFoundError('User', userId); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider reordering the current user check.
The current user existence check happens after deleting rejected requests (lines 154-160). While unlikely, if the authenticated user was somehow deleted between authentication and this point, you'd throw NotFoundError after already modifying data. Consider moving this check earlier or removing it since the user is already authenticated.
♻️ Suggested improvement
+ /* Get current user info for the response - do this early */
+ const currentUser = await db.user.findUnique({
+ where: { id: userId },
+ select: { id: true, username: true, displayName: true, avatarUrl: true },
+ });
+
+ if (!currentUser) {
+ throw new NotFoundError('User', userId);
+ }
+
/* Delete any previously REJECTED requests to allow resending */
await db.friendRequest.deleteMany({
where: {
fromId: userId,
toId: toUserId,
status: 'REJECTED',
},
});
- /* Get current user info for the response */
- const currentUser = await db.user.findUnique({
- where: { id: userId },
- select: { id: true, username: true, displayName: true, avatarUrl: true },
- });
-
- if (!currentUser) {
- throw new NotFoundError('User', userId);
- }
-
/* Create the friend request */Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @apps/server/src/routes/friendRequests.ts around lines 162 - 170, The
existence check for the authenticated user (the db.user.findUnique call that
assigns currentUser and the subsequent NotFoundError throw) must be moved
earlier in the handler to run before any state-changing operations (e.g., before
deleting rejected requests) so we never mutate data for a user that no longer
exists; locate the currentUser variable and NotFoundError usage in the friend
requests route/handler and move that check to run immediately after
authentication (or alternatively remove the redundant check if you can guarantee
authentication implies existence), ensuring all references to currentUser still
point to the validated result.
1 participant
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.