Conversation
Contributor
WalkthroughAdds a friends system: Zod API schemas, a client-side Friends API, a LitElement FriendsList UI with pagination and request actions, and AccountModal integration plus new English translations. ChangesFriends Feature Implementation
Sequence DiagramsequenceDiagram
participant User
participant FriendsList
participant FriendsApi
participant Server
User->>FriendsList: open friends tab
FriendsList->>FriendsApi: fetchFriendRequests()
FriendsList->>FriendsApi: fetchFriends(page, limit)
FriendsApi->>Server: GET /friends/requests
FriendsApi->>Server: GET /friends?page=X&limit=Y
Server-->>FriendsApi: requests / paginated friends
FriendsList->>User: render lists
User->>FriendsList: sendFriendRequest(publicId)
FriendsList->>FriendsApi: sendFriendRequest(publicId)
FriendsApi->>Server: POST /friends/requests/{publicId}
Server-->>FriendsApi: status (requested|accepted) or error
FriendsApi-->>FriendsList: response or FriendActionError
FriendsList->>FriendsList: update state, show toast
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/client/components/FriendsList.ts`:
- Around line 117-123: After mutating local lists (e.g., this.incoming,
this.friends, this.friendsTotal) in the accept/remove handlers, reset the
component's pagination cache/state and re-fetch the first page instead of only
editing the local arrays to avoid later duplicate/missing rows; specifically,
after the local update (the block that modifies this.incoming, this.friends and
this.friendsTotal), clear whatever pagination buffers you maintain (e.g., pages
array, nextCursor/currentPage) and invoke loadMore() (or a refreshFriends()
helper) to reload from the server so the client and paginated server state stay
in sync.
In `@src/core/ApiSchemas.ts`:
- Around line 221-248: Add unit tests that validate FriendEntrySchema,
FriendRequestsResponseSchema, FriendsListResponseSchema, and
SendFriendRequestResponseSchema: for FriendEntrySchema test a valid object with
publicId string and createdAt ISO datetime and invalid cases (missing publicId,
bad createdAt); for FriendRequestsResponseSchema test valid payloads where
incoming/outgoing are arrays of valid FriendEntry objects and invalid cases
(non-array, invalid entries); for FriendsListResponseSchema test a valid payload
with results array of FriendEntry objects and numeric total/page/limit and
invalid cases (missing/negative/non-numeric pagination fields, non-array
results, invalid entries); and for SendFriendRequestResponseSchema test accepted
valid enum values "requested" and "accepted" plus invalid string values. Use the
schema parse/validation methods in your test harness to assert success for valid
fixtures and thrown/failed validation for each invalid fixture, referencing the
schema symbols FriendEntrySchema, FriendRequestsResponseSchema,
FriendsListResponseSchema, and SendFriendRequestResponseSchema to locate the
code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a87f5d49-6e0e-4a30-828b-bc458b146182
📒 Files selected for processing (5)
resources/lang/en.jsonsrc/client/AccountModal.tssrc/client/FriendsApi.tssrc/client/components/FriendsList.tssrc/core/ApiSchemas.ts
github-project-automation
Bot
moved this from Triage
to Development
in OpenFront Release Management
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/core/ApiSchemas.ts (1)
221-248:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd tests for the new core friend schemas before merge.
These new
src/corecontracts were added without matching tests in the provided changes.As per coding guidelines: "All changes to
src/core/must include tests".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/ApiSchemas.ts` around lines 221 - 248, Add unit tests for the new core schemas: FriendEntrySchema, FriendRequestsResponseSchema, FriendsListResponseSchema, and SendFriendRequestResponseSchema. For each schema write tests that validate successful parsing of a correct example payload and that invalid payloads fail validation (missing required fields, wrong types, empty arrays where applicable); specifically test FriendEntrySchema for publicId (string) and createdAt (ISO datetime), FriendRequestsResponseSchema for incoming/outgoing arrays of FriendEntrySchema, FriendsListResponseSchema for results array plus numeric total/page/limit, and SendFriendRequestResponseSchema for the enum values "requested" and "accepted". Use the project’s existing test framework and schema parsing helpers to assert both .parse() success and that .safeParse()/errors catch invalid inputs.
🧹 Nitpick comments (1)
src/client/FriendsApi.ts (1)
55-63: ⚡ Quick winUse
friendsFetchinfetchFriendsto avoid request drift.
fetchFriendsduplicates auth/header logic instead of using the shared helper. Using one path is safer and easier to maintain.Suggested refactor
- const res = await fetch(url.toString(), { - headers: { - Accept: "application/json", - Authorization: await getAuthHeader(), - }, - }); + const res = await friendsFetch( + `/friends?page=${encodeURIComponent(String(page))}&limit=${encodeURIComponent(String(limit))}`, + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/client/FriendsApi.ts` around lines 55 - 63, The fetchFriends function is constructing the URL and duplicating auth/header logic instead of using the shared friendsFetch helper; update fetchFriends to call friendsFetch for the "/friends" endpoint (passing page and limit as query params or as the helper's expected args) and remove the manual fetch/Authorization header code. Locate fetchFriends and replace the new URL + fetch(...) block with a single await to friendsFetch (or friendsFetch.get/post depending on the helper API), then continue using the returned response as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/ApiSchemas.ts`:
- Around line 235-240: FriendsListResponseSchema currently uses z.number() for
total, page, and limit allowing floats and negatives; update
FriendsListResponseSchema to use z.number().int().min(...) with total:
.int().min(0), page: .int().min(1), limit: .int().min(1) so only valid integers
pass, and adjust any imports if needed; add unit tests for
FriendsListResponseSchema that assert valid payloads pass and invalid cases
(negative or zero/negative page/limit, non-integers for total/page/limit) fail
validation to cover the three constraints (reference FriendsListResponseSchema
and FriendEntrySchema in tests).
---
Duplicate comments:
In `@src/core/ApiSchemas.ts`:
- Around line 221-248: Add unit tests for the new core schemas:
FriendEntrySchema, FriendRequestsResponseSchema, FriendsListResponseSchema, and
SendFriendRequestResponseSchema. For each schema write tests that validate
successful parsing of a correct example payload and that invalid payloads fail
validation (missing required fields, wrong types, empty arrays where
applicable); specifically test FriendEntrySchema for publicId (string) and
createdAt (ISO datetime), FriendRequestsResponseSchema for incoming/outgoing
arrays of FriendEntrySchema, FriendsListResponseSchema for results array plus
numeric total/page/limit, and SendFriendRequestResponseSchema for the enum
values "requested" and "accepted". Use the project’s existing test framework and
schema parsing helpers to assert both .parse() success and that
.safeParse()/errors catch invalid inputs.
---
Nitpick comments:
In `@src/client/FriendsApi.ts`:
- Around line 55-63: The fetchFriends function is constructing the URL and
duplicating auth/header logic instead of using the shared friendsFetch helper;
update fetchFriends to call friendsFetch for the "/friends" endpoint (passing
page and limit as query params or as the helper's expected args) and remove the
manual fetch/Authorization header code. Locate fetchFriends and replace the new
URL + fetch(...) block with a single await to friendsFetch (or
friendsFetch.get/post depending on the helper API), then continue using the
returned response as before.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6b12b3c6-e6d7-4b68-a41b-1d0a3dbfe826
📒 Files selected for processing (5)
resources/lang/en.jsonsrc/client/AccountModal.tssrc/client/FriendsApi.tssrc/client/components/FriendsList.tssrc/core/ApiSchemas.ts
✅ Files skipped from review due to trivial changes (1)
- resources/lang/en.json
github-project-automation
Bot
moved this from Development
to Complete
in OpenFront Release Management
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.
Description:
Add Friends tab to Account modal
Summary
<friends-list>Lit component covering the full friend lifecycle: send request, accept / deny incoming, withdraw outgoing, remove friend, paginated list with "Load more".FriendsApi.tswrappingGET/POST/DELETE /friends*endpoints with typed error codes (not_found/conflict/bad_request/request_failed).core/ApiSchemas.ts.friends.*block inen.json.Friends and pending requests are displayed by public ID via
copy-button, matching the existing clan convention.Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
evan