Added a new fun pfp avatar selection #103
Conversation
WalkthroughThis update refactors the user rating system across backend and frontend, renaming "EloRating" to "Rating" and shifting rating calculations from integer-based Elo to float64 precision. It introduces avatar URL support in user profiles, adds an avatar selection modal in the frontend, updates profile and leaderboard logic, and enhances user data handling in debates. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend
participant Backend
participant DB
User->>Frontend: Open Profile Page
Frontend->>Backend: GET /user/profile (with token)
Backend->>DB: Fetch user profile
DB-->>Backend: Return user data (with Rating, avatarUrl)
Backend-->>Frontend: Return profile JSON
Frontend-->>User: Display profile (with avatar and rating)
User->>Frontend: Click "Edit Avatar"
Frontend->>Frontend: Open AvatarModal
User->>Frontend: Select avatar and save
Frontend->>Backend: PUT /user/updateprofile (with avatarUrl)
Backend->>DB: Update user avatarUrl
DB-->>Backend: Ack
Backend-->>Frontend: Success response
Frontend-->>User: Update avatar in UI
Possibly related PRs
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Nitpick comments (1)
frontend/src/Pages/BotSelection.tsx (1)
323-329: Make guest user avatar generation more robustThe hardcoded Dicebear URL with version 9.x might break if the API changes. Consider using a more stable approach.
user: user || { email: "", firstName: "Guest", lastName: "", - avatarUrl: "https://api.dicebear.com/9.x/big-ears/svg?seed=Guest", + avatarUrl: `https://api.dicebear.com/7.x/avataaars/svg?seed=${Date.now()}`, eloRating: 1500, },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (20)
backend/controllers/auth.go(2 hunks)backend/controllers/leaderboard.go(2 hunks)backend/controllers/profile_controller.go(6 hunks)backend/models/debate.go(1 hunks)backend/rating/glicko2.go(3 hunks)backend/routes/debate.go(2 hunks)backend/routes/leaderboard.go(0 hunks)backend/routes/rooms.go(2 hunks)backend/services/rating_service.go(4 hunks)backend/utils/auth.go(0 hunks)backend/utils/debate.go(1 hunks)backend/utils/populate.go(1 hunks)backend/utils/user.go(2 hunks)frontend/package.json(1 hunks)frontend/src/Pages/BotSelection.tsx(9 hunks)frontend/src/Pages/DebateRoom.tsx(19 hunks)frontend/src/Pages/Profile.tsx(8 hunks)frontend/src/components/AvatarModal.tsx(1 hunks)frontend/src/components/ui/slider.tsx(1 hunks)frontend/src/services/profileService.ts(1 hunks)
💤 Files with no reviewable changes (2)
- backend/utils/auth.go
- backend/routes/leaderboard.go
🧰 Additional context used
🧬 Code Graph Analysis (6)
backend/routes/debate.go (1)
backend/db/db.go (1)
MongoDatabase(17-17)
frontend/src/components/ui/slider.tsx (1)
frontend/src/lib/utils.ts (1)
cn(4-6)
backend/services/rating_service.go (3)
backend/rating/glicko2.go (2)
Glicko2(51-53)Player(21-26)backend/models/debate.go (1)
Debate(10-25)backend/db/db.go (1)
MongoDatabase(17-17)
backend/utils/populate.go (5)
backend/config/config.go (1)
LoadConfig(53-65)backend/db/db.go (1)
MongoDatabase(17-17)backend/services/rating_service.go (2)
InitRatingService(18-20)GetRatingSystem(22-24)backend/models/user.go (1)
User(10-30)backend/rating/glicko2.go (1)
Config(29-36)
frontend/src/Pages/BotSelection.tsx (2)
frontend/src/utils/auth.ts (1)
getAuthToken(5-7)frontend/src/services/profileService.ts (1)
getProfile(3-15)
frontend/src/components/AvatarModal.tsx (3)
frontend/src/components/ui/button.tsx (1)
Button(57-57)frontend/src/components/ui/label.tsx (1)
Label(24-24)frontend/src/components/ui/slider.tsx (1)
Slider(26-26)
🪛 golangci-lint (1.64.8)
backend/controllers/profile_controller.go
72-72: composites: go.mongodb.org/mongo-driver/bson/primitive.E struct literal uses unkeyed fields
(govet)
116-116: composites: go.mongodb.org/mongo-driver/bson/primitive.E struct literal uses unkeyed fields
(govet)
245-245: Error return value of (*go.mongodb.org/mongo-driver/mongo.Collection).UpdateOne is not checked
(errcheck)
246-246: Error return value of (*go.mongodb.org/mongo-driver/mongo.Collection).UpdateOne is not checked
(errcheck)
250-250: Error return value of (*go.mongodb.org/mongo-driver/mongo.Collection).InsertOne is not checked
(errcheck)
258-258: Error return value of (*go.mongodb.org/mongo-driver/mongo.Collection).InsertOne is not checked
(errcheck)
🪛 Biome (1.9.4)
frontend/src/Pages/DebateRoom.tsx
[error] 375-375: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
🔇 Additional comments (20)
frontend/package.json (1)
22-22: ✅ Dependency Version ValidatedThe
@radix-ui/react-sliderentry at frontend/package.json:22 (^1.3.5) is the latest stable release and has no publicly reported security vulnerabilities. No further action is needed—this addition is safe to merge.backend/controllers/auth.go (1)
147-147: Consistent field renaming applied.The rating field renaming is properly applied here as well, maintaining the same initialization logic and value.
backend/utils/user.go (1)
24-24: Field rename is consistent and correct.The change from
EloRatingtoRatingis properly applied across all test users, maintaining the same rating values and structure.Also applies to: 32-32, 40-40
backend/models/debate.go (1)
13-13: Verify database documents and API consumers for theI confirmed there are no remaining references to
UserEmailor"userEmail"in the Go codebase. The struct change itself is clean, but you’ll need to ensure that:
- Existing MongoDB documents with a
"userEmail"key are migrated or handled (e.g. via a migration script).- API clients and documentation are updated to expect
"email"instead of"userEmail".- Any frontend or downstream services consuming this endpoint are aligned with the new field name.
Please verify these points before merging.
backend/utils/debate.go (1)
30-30: Consistent field renaming applied across all sample data.The systematic change from
EloChangetoRatingChangeis properly applied across all sample debate entries. The values and logic remain unchanged, maintaining data integrity while aligning with the broader rating system refactoring.Also applies to: 37-37, 44-44, 51-51, 58-58
backend/routes/rooms.go (1)
67-67: LGTM: Consistent field renaming maintains database compatibility.The renaming from
EloRatingtoRatingis well-implemented, preserving the BSON tag"eloRating"for database compatibility while updating the internal field name. This aligns with the broader refactoring across the codebase.Also applies to: 80-80
backend/routes/debate.go (2)
4-4: LGTM: Import changes support standardized database access pattern.The addition of "context" and "arguehub/db" imports correctly supports the updated database access pattern used in the collection retrieval.
Also applies to: 9-9
60-61: LGTM: Standardized database access pattern with proper context usage.The change from
db.GetCollectionto directdb.MongoDatabase.Collection("debates")access standardizes the database interaction pattern. Usingcontext.Background()is appropriate for this operation.backend/controllers/leaderboard.go (1)
49-49: Confirm acceptable precision loss for leaderboard ratingsThe
Ratingfield is now afloat64(backend/models/user.go:15). Converting it withint(user.Rating)in the leaderboard controller (backend/controllers/leaderboard.go:86) will truncate any fractional component, potentially losing precision in displayed scores. Please confirm that dropping the decimal part is intentional and acceptable for the leaderboard.
- backend/models/user.go:15:
Rating float64- backend/controllers/leaderboard.go:86:
int(user.Rating)frontend/src/services/profileService.ts (1)
23-24: LGTM: Avatar functionality properly implemented.The addition of the optional
avatarUrlparameter and its inclusion in the request body correctly implements the avatar selection feature. The optional parameter maintains backward compatibility.Also applies to: 32-32
backend/rating/glicko2.go (1)
52-52: LGTM: Config field visibility change enables external access.The change from private
configto publicConfigfield is correctly implemented with all internal references updated accordingly. This enables external packages to access the rating system configuration while maintaining the same functionality.Also applies to: 60-60, 66-68, 109-109, 115-115, 121-121, 126-126, 171-171
backend/services/rating_service.go (2)
66-66: LGTM! Field name correctly updatedThe field name change from
UserEmailto
94-94: Direct MongoDB collection access is consistentThe change from
db.GetCollection("users")todb.MongoDatabase.Collection("users")is consistent with the codebase refactoring pattern.Also applies to: 101-101
frontend/src/components/ui/slider.tsx (1)
1-26: Well-structured slider componentThe slider component is properly implemented using Radix UI primitives with appropriate styling and React patterns.
backend/utils/populate.go (1)
30-32: Rating system initialization pattern looks goodThe changes to use
InitRatingServiceandGetRatingSystemfollow the new singleton pattern, and accessingConfigas a public field is consistent with the struct changes.Also applies to: 39-41, 48-50
frontend/src/Pages/BotSelection.tsx (1)
292-301: Good validation and error handlingThe added validation for bot and topic selection, along with disabling the button during loading, improves the user experience.
Also applies to: 559-559
frontend/src/Pages/Profile.tsx (1)
195-231: Well-implemented avatar management with optimistic updatesThe avatar selection feature with optimistic updates and proper error handling provides a good user experience. The modal integration is clean and the rollback on error is a nice touch.
Also applies to: 543-556
frontend/src/Pages/DebateRoom.tsx (1)
606-607: Good integration of user profile data.The component now properly uses the user's avatar URL and display name from the debate data, which aligns well with the backend changes for user profile management.
Also applies to: 672-683
frontend/src/components/AvatarModal.tsx (1)
287-323: Excellent performance optimization with memoized previews.The use of
useMemoto cache preview URLs prevents unnecessary re-renders and API calls when showing style options. This is a well-thought-out optimization for a component with many preview images.backend/controllers/profile_controller.go (1)
21-30: Well-implemented Elo rating calculation.The new
calculateEloRatingfunction correctly implements the standard Elo rating formula with float64 precision, which provides better accuracy than integer-based calculations. The K-factor of 32 is appropriate for a general rating system.
| Password: "", // No password for Google users | ||
| Nickname: nickname, | ||
| EloRating: 1200, | ||
| Rating: 1200, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Field rename looks good - verify consistency across codebase.
The change from EloRating to Rating is consistent with the broader refactoring effort. The initialization value of 1200 remains unchanged, which preserves existing functionality.
🏁 Script executed:
#!/bin/bash
# Verify that all references to EloRating have been updated to Rating
rg -i "elorating" --type goLength of output: 1213
Remaining EloRating references need to be updated or justified
I ran a global search and found several places where EloRating still appears in code or BSON mappings:
• backend/routes/rooms.go
– Struct tag uses bson:"eloRating" on Rating field
• backend/controllers/leaderboard.go
– Sort key {"eloRating", -1}
• backend/controllers/profile_controller.go
– Function name calculateEloRating(...)
– Sort key SetSort(bson.D{{"eloRating", -1}})
– Struct tag bson:"eloRating" on Rating
– Update payloads and query filters still reference "eloRating"
Please either:
- Fully rename these references to use
Rating(and, if you’re changing the MongoDB field name, plan a migration), - Or clarify why the BSON field and helper function must retain
eloRating/calculateEloRating.
— once resolved, we can approve the refactor.
🤖 Prompt for AI Agents
In backend/controllers/auth.go at line 78 and related files
backend/routes/rooms.go, backend/controllers/leaderboard.go, and
backend/controllers/profile_controller.go, the code still uses the old
`EloRating` naming in BSON tags, sort keys, function names, and update payloads.
You need to consistently rename all these references from `EloRating` or
`eloRating` to `Rating` and update the BSON field names accordingly. If changing
the MongoDB field name, plan and implement a migration to update existing
documents. Alternatively, if you decide to keep the old naming for
compatibility, add comments explaining why these references remain unchanged.
Ensure all related code and database mappings are aligned for consistency.
|
|
||
| // PopulateTestUsers inserts sample users into the database | ||
| func PopulateTestUsers() { | ||
| func PopulateTestUsers1() { |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Questionable function rename - consider better naming.
The function name change from PopulateTestUsers to PopulateTestUsers1 is concerning. The "1" suffix doesn't provide meaningful context and suggests incomplete refactoring.
Consider either:
- Using a more descriptive name like
PopulateTestUsersV2orPopulateTestUsersWithRating - Keeping the original name if this is a replacement
- Documenting why the "1" suffix is needed
🏁 Script executed:
#!/bin/bash
# Check if there are other PopulateTestUsers functions and where this function is called
rg "PopulateTestUsers" --type go -A 2 -B 2Length of output: 901
Clarify and rename PopulateTestUsers1 to match its intent and usage
The new PopulateTestUsers1 in backend/utils/user.go currently coexists with the original PopulateTestUsers in backend/utils/populate.go and isn’t wired up in backend/cmd/server/main.go. The 1 suffix is non-descriptive and hides this implementation from call sites.
Locations to review:
backend/utils/user.go
func PopulateTestUsers1() { … }backend/utils/populate.go
func PopulateTestUsers() { … }backend/cmd/server/main.go
utils.PopulateTestUsers()
Suggested fixes:
- If the new logic should replace the old one:
• RenamePopulateTestUsers1()→PopulateTestUsers()inuser.goand remove or refactor the legacy implementation inpopulate.go. - If both behaviours are needed:
• Use descriptive names (e.g.,PopulateTestUsersWithRating/PopulateLegacyTestUsers).
• Updatemain.goto call the correct function. - Add a brief comment/doc explaining why each variant exists.
🤖 Prompt for AI Agents
In backend/utils/user.go at line 14, rename the function PopulateTestUsers1 to a
more descriptive name that reflects its purpose, such as
PopulateTestUsersWithRating or another meaningful name. Review
backend/utils/populate.go for the existing PopulateTestUsers function and decide
if the new function should replace it or coexist. If replacing, rename
PopulateTestUsers1 to PopulateTestUsers and remove or refactor the old function
accordingly. If both are needed, keep both with clear, distinct names and update
backend/cmd/server/main.go to call the appropriate function. Add comments to
each function explaining their differences and usage.
| func GetRatingSystem() *rating.Glicko2 { | ||
| return ratingSystem | ||
| } |
There was a problem hiding this comment.
Add nil check in GetRatingSystem
The function could return nil if called before InitRatingService. Consider adding a nil check or initializing the rating system lazily.
func GetRatingSystem() *rating.Glicko2 {
+ if ratingSystem == nil {
+ panic("rating system not initialized")
+ }
return ratingSystem
}📝 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.
| func GetRatingSystem() *rating.Glicko2 { | |
| return ratingSystem | |
| } | |
| func GetRatingSystem() *rating.Glicko2 { | |
| if ratingSystem == nil { | |
| panic("rating system not initialized") | |
| } | |
| return ratingSystem | |
| } |
🤖 Prompt for AI Agents
In backend/services/rating_service.go around lines 22 to 24, the GetRatingSystem
function may return nil if called before InitRatingService initializes the
rating system. To fix this, add a nil check inside GetRatingSystem and
initialize the rating system lazily if it is nil before returning it.
| cfg, err := config.LoadConfig("./config/config.prod.yml") | ||
| if err != nil { | ||
| log.Fatalf("Failed to load config: %v", err) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reconsider loading production config for test data
Loading the production configuration file in a utility function that creates test users could lead to issues. Consider using a test-specific configuration or making the config path configurable.
-func PopulateTestUsers() {
- cfg, err := config.LoadConfig("./config/config.prod.yml")
+func PopulateTestUsers(configPath string) {
+ if configPath == "" {
+ configPath = "./config/config.test.yml"
+ }
+ cfg, err := config.LoadConfig(configPath)🤖 Prompt for AI Agents
In backend/utils/populate.go around lines 18 to 21, the code loads the
production config file which is not suitable for test data creation. Modify the
function to accept a configurable config path parameter or switch to loading a
test-specific configuration file instead of the production one to avoid
environment conflicts.
| const [user, setUser] = useState<{ | ||
| email: string; | ||
| displayName: string; | ||
| avatarUrl: string; | ||
| eloRating: number; | ||
| } | null>(null); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Field name inconsistency with backend
The user state uses eloRating but according to the AI summary, the backend has standardized on Rating. This inconsistency could cause issues.
Also applies to: 242-242
🏁 Script executed:
#!/bin/bash
# Check what field name the backend actually returns
rg -A 10 "type.*Profile.*struct" --type go
rg "json:.*rating" --type goLength of output: 496
Inconsistent rating field name – update to match backend’s rating key
The frontend’s user state uses eloRating, but the backend’s JSON uses rating. Rename all occurrences of eloRating to rating to ensure the fetched profile maps correctly.
• File: frontend/src/Pages/BotSelection.tsx
– Lines 213–218, 242: update the useState type and any user.eloRating accesses
• Replace hard-coded guest avatar URL with a shared constant or placeholder
Proposed diff:
--- a/frontend/src/Pages/BotSelection.tsx
+++ b/frontend/src/Pages/BotSelection.tsx
@@ -213,7 +213,7 @@ export function BotSelection() {
- const [user, setUser] = useState<{
- email: string;
- displayName: string;
- avatarUrl: string;
- eloRating: number;
- } | null>(null);
+ const [user, setUser] = useState<{
+ email: string;
+ displayName: string;
+ avatarUrl: string;
+ rating: number;
+ } | null>(null);
useEffect(() => {
// …
@@ -242,7 +242,7 @@ export function BotSelection() {
fallbackUser = {
email: '',
displayName: 'Guest',
- avatarUrl: 'https://api.adorable.io/avatars/50/guest.png',
- eloRating: 1200,
+ avatarUrl: DEFAULT_AVATAR_URL,
+ rating: 1200,
};
setUser(fallbackUser);
}, []);– Ensure all user.eloRating usages are updated to user.rating
– Define DEFAULT_AVATAR_URL (e.g. in constants) instead of inlining it
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In frontend/src/Pages/BotSelection.tsx around lines 213 to 218 and line 242,
rename the user state field from eloRating to rating to match the backend JSON
key. Update all references from user.eloRating to user.rating accordingly.
Additionally, replace the hard-coded guest avatar URL with a shared constant or
placeholder by defining DEFAULT_AVATAR_URL in a constants file or at the top of
this file and use that constant instead of the inline string.
| className="text-sm w-full resize-none h-20" | ||
| /> | ||
| <div className="flex gap-2"> | ||
| <div className="flex gap- personally2"> |
There was a problem hiding this comment.
Fix className typo
There's a typo in the className that will cause styling issues.
- <div className="flex gap- personally2">
+ <div className="flex gap-2">📝 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.
| <div className="flex gap- personally2"> | |
| <div className="flex gap-2"> |
🤖 Prompt for AI Agents
In frontend/src/Pages/Profile.tsx at line 351, there is a typo in the className
attribute "gap- personally2" which will cause styling issues. Correct the
className by fixing the typo to a valid Tailwind CSS class or intended class
name, ensuring proper spacing and syntax for the styling to apply correctly.
| isBotTurn: | ||
| prevState.userStance === phaseSequences[newPhase][0] ? false : true, | ||
| timer: phases[newPhase].time, |
There was a problem hiding this comment.
Simplify the ternary expression.
The static analysis correctly identifies an unnecessary use of boolean literals. The expression already evaluates to a boolean.
- isBotTurn:
- prevState.userStance === phaseSequences[newPhase][0] ? false : true,
+ isBotTurn: prevState.userStance !== phaseSequences[newPhase][0],📝 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.
| isBotTurn: | |
| prevState.userStance === phaseSequences[newPhase][0] ? false : true, | |
| timer: phases[newPhase].time, | |
| isBotTurn: prevState.userStance !== phaseSequences[newPhase][0], | |
| timer: phases[newPhase].time, |
🧰 Tools
🪛 Biome (1.9.4)
[error] 375-375: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
🤖 Prompt for AI Agents
In frontend/src/Pages/DebateRoom.tsx around lines 374 to 376, simplify the
ternary expression that sets isBotTurn by removing the unnecessary true/false
literals. Replace the expression with a direct boolean comparison that returns
the same result without the explicit ternary operator.
| const [seed, setSeed] = useState( | ||
| currentAvatar?.split('seed=')[1]?.split('&')[0] || 'Jude' | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for seed extraction.
The current seed extraction from the avatar URL could fail if the URL format is unexpected or if currentAvatar is malformed.
- const [seed, setSeed] = useState(
- currentAvatar?.split('seed=')[1]?.split('&')[0] || 'Jude'
- );
+ const [seed, setSeed] = useState(() => {
+ if (!currentAvatar) return 'Jude';
+ try {
+ const url = new URL(currentAvatar);
+ return url.searchParams.get('seed') || 'Jude';
+ } catch {
+ return 'Jude';
+ }
+ });📝 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.
| const [seed, setSeed] = useState( | |
| currentAvatar?.split('seed=')[1]?.split('&')[0] || 'Jude' | |
| ); | |
| const [seed, setSeed] = useState(() => { | |
| if (!currentAvatar) return 'Jude'; | |
| try { | |
| const url = new URL(currentAvatar); | |
| return url.searchParams.get('seed') || 'Jude'; | |
| } catch { | |
| return 'Jude'; | |
| } | |
| }); |
🤖 Prompt for AI Agents
In frontend/src/components/AvatarModal.tsx around lines 20 to 22, the seed
extraction from currentAvatar assumes a specific URL format and can fail if the
format is unexpected or currentAvatar is malformed. Add error handling by safely
checking if currentAvatar contains 'seed=' and properly extracting the seed
value without causing runtime errors. Use conditional checks or try-catch to
handle unexpected formats and provide a default seed value if extraction fails.
| cursor, err := db.MongoDatabase.Collection("users").Find( | ||
| dbCtx, | ||
| bson.M{}, | ||
| options.Find().SetSort(bson.D{{"eloRating", -1}}).SetLimit(5), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use keyed fields in BSON documents.
The static analysis identifies unkeyed struct fields which can lead to maintenance issues if the field order changes.
- options.Find().SetSort(bson.D{{"eloRating", -1}}).SetLimit(5),
+ options.Find().SetSort(bson.D{{Key: "eloRating", Value: -1}}).SetLimit(5),- options.Find().SetSort(bson.D{{"date", 1}}),
+ options.Find().SetSort(bson.D{{Key: "date", Value: 1}}),Also applies to: 116-116
🧰 Tools
🪛 golangci-lint (1.64.8)
72-72: composites: go.mongodb.org/mongo-driver/bson/primitive.E struct literal uses unkeyed fields
(govet)
🤖 Prompt for AI Agents
In backend/controllers/profile_controller.go at lines 72 and 116, replace the
unkeyed BSON document fields with keyed fields by explicitly specifying the
field names and values using bson.D{{Key: "eloRating", Value: -1}} instead of
bson.D{{"eloRating", -1}}. This ensures better maintainability and clarity by
avoiding reliance on field order.
| db.MongoDatabase.Collection("users").UpdateOne(dbCtx, bson.M{"_id": winnerID}, bson.M{"$set": bson.M{"eloRating": newWinnerElo}}) | ||
| db.MongoDatabase.Collection("users").UpdateOne(dbCtx, bson.M{"_id": loserID}, bson.M{"$set": bson.M{"eloRating": newLoserElo}}) |
There was a problem hiding this comment.
Critical: Handle database operation errors.
The static analysis correctly identifies that database update and insert operations are not checking for errors. This could lead to silent failures where Elo ratings are calculated but not persisted, or debate history is incomplete.
- db.MongoDatabase.Collection("users").UpdateOne(dbCtx, bson.M{"_id": winnerID}, bson.M{"$set": bson.M{"eloRating": newWinnerElo}})
- db.MongoDatabase.Collection("users").UpdateOne(dbCtx, bson.M{"_id": loserID}, bson.M{"$set": bson.M{"eloRating": newLoserElo}})
+ if _, err := db.MongoDatabase.Collection("users").UpdateOne(dbCtx, bson.M{"_id": winnerID}, bson.M{"$set": bson.M{"eloRating": newWinnerElo}}); err != nil {
+ ctx.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to update winner rating"})
+ return
+ }
+ if _, err := db.MongoDatabase.Collection("users").UpdateOne(dbCtx, bson.M{"_id": loserID}, bson.M{"$set": bson.M{"eloRating": newLoserElo}}); err != nil {
+ ctx.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to update loser rating"})
+ return
+ }Similarly, add error handling for the InsertOne operations:
- db.MongoDatabase.Collection("debates").InsertOne(dbCtx, bson.M{
+ if _, err := db.MongoDatabase.Collection("debates").InsertOne(dbCtx, bson.M{
"email": winner.Email,
"topic": req.Topic,
"result": "win",
"eloChange": winnerChange,
"eloRating": newWinnerElo,
"date": now,
- })
+ }); err != nil {
+ // Log error but don't fail the request since ratings were updated
+ // Consider implementing a transaction for atomicity
+ }Also applies to: 250-265
🧰 Tools
🪛 golangci-lint (1.64.8)
245-245: Error return value of (*go.mongodb.org/mongo-driver/mongo.Collection).UpdateOne is not checked
(errcheck)
246-246: Error return value of (*go.mongodb.org/mongo-driver/mongo.Collection).UpdateOne is not checked
(errcheck)
🤖 Prompt for AI Agents
In backend/controllers/profile_controller.go around lines 245 to 246 and also
lines 250 to 265, the database update and insert operations are missing error
handling, which can cause silent failures. Modify each UpdateOne and InsertOne
call to capture the returned error, check if it is non-nil, and handle it
appropriately by logging the error or returning it to ensure failures are not
ignored and data consistency is maintained.
2 participants
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Style
Chores