Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
- Updated getsentry/prevent-action to use @v0 as recommended - Kept OpenID Connect authentication setup - Ready to test the integration
❌ 52 Tests Failed:
View the top 3 failed test(s) by shortest run time
|
Owner
Author
|
@sentry review |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| import { getServerSession } from 'next-auth'; | ||
| import { authOptions } from '@/lib/auth.config'; | ||
| import { PrismaClient, AchievementType } from '@prisma/client'; | ||
|
|
There was a problem hiding this comment.
Multiple PrismaClient instances are being created throughout the codebase. In production, this can cause connection pool exhaustion. Create a singleton instance in a shared module (e.g., src/lib/prisma.ts) and import it instead of creating new instances in each route handler.
Severity: HIGH
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: app/src/app/api/achievements/route.ts#L5
Potential issue: Multiple PrismaClient instances are being created throughout the
codebase. In production, this can cause connection pool exhaustion. Create a singleton
instance in a shared module (e.g., `src/lib/prisma.ts`) and import it instead of
creating new instances in each route handler.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 7293328
Comment on lines
+8
to
+64
| // Achievement definitions with metadata | ||
| const ACHIEVEMENT_DEFINITIONS = { | ||
| [AchievementType.STREAK_3_DAYS]: { | ||
| name: 'Streak Starter', | ||
| description: 'Work out for 3 days in a row', | ||
| icon: '🔥', | ||
| color: '#ff6b35', | ||
| gradient: 'linear-gradient(135deg, #ff6b35, #f7931e)' | ||
| }, | ||
| [AchievementType.STREAK_5_DAYS]: { | ||
| name: 'Consistency King', | ||
| description: 'Work out for 5 days in a row', | ||
| icon: '👑', | ||
| color: '#ffd700', | ||
| gradient: 'linear-gradient(135deg, #ffd700, #ffed4e)' | ||
| }, | ||
| [AchievementType.STREAK_7_DAYS]: { | ||
| name: 'Week Warrior', | ||
| description: 'Work out for 7 days in a row', | ||
| icon: '⚔️', | ||
| color: '#8b5cf6', | ||
| gradient: 'linear-gradient(135deg, #8b5cf6, #a855f7)' | ||
| }, | ||
| [AchievementType.STREAK_14_DAYS]: { | ||
| name: 'Fortnight Fighter', | ||
| description: 'Work out for 14 days in a row', | ||
| icon: '🛡️', | ||
| color: '#06b6d4', | ||
| gradient: 'linear-gradient(135deg, #06b6d4, #0891b2)' | ||
| }, | ||
| [AchievementType.STREAK_30_DAYS]: { | ||
| name: 'Monthly Master', | ||
| description: 'Work out for 30 days in a row', | ||
| icon: '🏆', | ||
| color: '#10b981', | ||
| gradient: 'linear-gradient(135deg, #10b981, #059669)' | ||
| }, | ||
| [AchievementType.FIRST_WORKOUT]: { | ||
| name: 'First Steps', | ||
| description: 'Complete your first workout', | ||
| icon: '🎯', | ||
| color: '#3b82f6', | ||
| gradient: 'linear-gradient(135deg, #3b82f6, #1d4ed8)' | ||
| }, | ||
| [AchievementType.WORKOUT_MARATHON]: { | ||
| name: 'Marathon Master', | ||
| description: 'Complete 26 workouts', | ||
| icon: '🏃♂️', | ||
| color: '#ef4444', | ||
| gradient: 'linear-gradient(135deg, #ef4444, #dc2626)' | ||
| }, | ||
| [AchievementType.CONSISTENCY_CHAMPION]: { | ||
| name: 'Consistency Champion', | ||
| description: 'Work out 4+ days in a week', | ||
| icon: '💪', | ||
| color: '#f59e0b', | ||
| gradient: 'linear-gradient(135deg, #f59e0b, #d97706)' |
There was a problem hiding this comment.
Achievement metadata (ACHIEVEMENT_DEFINITIONS) is hardcoded in the route handler. This creates maintainability issues - the same definitions are needed in the components and services. Extract this to a shared constants file (e.g., src/lib/achievementConstants.ts) that can be imported by all modules needing these definitions.
Severity: MEDIUM
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: app/src/app/api/achievements/route.ts#L8-L64
Potential issue: Achievement metadata (ACHIEVEMENT_DEFINITIONS) is hardcoded in the
route handler. This creates maintainability issues - the same definitions are needed in
the components and services. Extract this to a shared constants file (e.g.,
`src/lib/achievementConstants.ts`) that can be imported by all modules needing these
definitions.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 7293328
Comment on lines
+97
to
+103
| console.error('Error fetching achievements:', error); | ||
| return NextResponse.json( | ||
| { error: 'Failed to fetch achievements' }, | ||
| { status: 500 } | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
The error handling in the catch block doesn't distinguish between different error types (validation errors, database errors, auth errors). Consider adding more specific error handling and logging to help with debugging. Also, ensure sensitive information (like full error objects) is not exposed to clients.
Severity: MEDIUM
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: app/src/app/api/achievements/route.ts#L97-L103
Potential issue: The error handling in the catch block doesn't distinguish between
different error types (validation errors, database errors, auth errors). Consider adding
more specific error handling and logging to help with debugging. Also, ensure sensitive
information (like full error objects) is not exposed to clients.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 7293328
Comment on lines
+170
to
+176
| } | ||
| }); | ||
| } | ||
| } catch (error) { | ||
| console.error('Error creating/updating achievement:', error); | ||
| return NextResponse.json( | ||
| { error: 'Failed to create/update achievement' }, |
There was a problem hiding this comment.
Similar error handling issue in POST method catch block. The generic 'Failed to create/update achievement' message makes it difficult to debug issues. Consider logging the specific error type and providing more context in error responses.
Severity: MEDIUM
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: app/src/app/api/achievements/route.ts#L170-L176
Potential issue: Similar error handling issue in POST method catch block. The generic
'Failed to create/update achievement' message makes it difficult to debug issues.
Consider logging the specific error type and providing more context in error responses.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 7293328
| @@ -0,0 +1,214 @@ | |||
| import { PrismaClient, AchievementType } from '@prisma/client'; | |||
|
|
|||
| const prisma = new PrismaClient(); | |||
There was a problem hiding this comment.
PrismaClient instance should be created as a singleton to avoid connection pool exhaustion. Move this to a shared module and import it instead of creating a new instance here.
Severity: HIGH
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: app/src/lib/achievementService.ts#L3
Potential issue: PrismaClient instance should be created as a singleton to avoid
connection pool exhaustion. Move this to a shared module and import it instead of
creating a new instance here.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 7293328
| @@ -2,6 +2,7 @@ import { NextRequest, NextResponse } from "next/server"; | |||
| import { getServerSession } from "next-auth"; | |||
| import { authOptions } from "@/lib/auth.config"; | |||
| import { PrismaClient, ActivityType, EffortLevel } from "@prisma/client"; | |||
There was a problem hiding this comment.
Multiple PrismaClient instances throughout the codebase create connection pool issues. Use a shared singleton instance instead of creating new instances in each route.
Severity: HIGH
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: app/src/app/api/activities/route.ts#L4
Potential issue: Multiple PrismaClient instances throughout the codebase create
connection pool issues. Use a shared singleton instance instead of creating new
instances in each route.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 7293328
Comment on lines
+1
to
+87
| import { AchievementType } from '@prisma/client'; | ||
|
|
||
| describe('Achievement System', () => { | ||
| describe('Achievement Types', () => { | ||
| it('should have all required achievement types', () => { | ||
| expect(AchievementType.STREAK_3_DAYS).toBe('STREAK_3_DAYS'); | ||
| expect(AchievementType.STREAK_5_DAYS).toBe('STREAK_5_DAYS'); | ||
| expect(AchievementType.STREAK_7_DAYS).toBe('STREAK_7_DAYS'); | ||
| expect(AchievementType.STREAK_14_DAYS).toBe('STREAK_14_DAYS'); | ||
| expect(AchievementType.STREAK_30_DAYS).toBe('STREAK_30_DAYS'); | ||
| expect(AchievementType.FIRST_WORKOUT).toBe('FIRST_WORKOUT'); | ||
| expect(AchievementType.WORKOUT_MARATHON).toBe('WORKOUT_MARATHON'); | ||
| expect(AchievementType.CONSISTENCY_CHAMPION).toBe('CONSISTENCY_CHAMPION'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('Achievement Logic', () => { | ||
| it('should calculate streak correctly for consecutive days', () => { | ||
| const today = new Date(); | ||
| const yesterday = new Date(today); | ||
| yesterday.setDate(yesterday.getDate() - 1); | ||
| const twoDaysAgo = new Date(today); | ||
| twoDaysAgo.setDate(twoDaysAgo.getDate() - 2); | ||
|
|
||
| // Test that we can create date arrays for streak calculation | ||
| const activities = [today, yesterday, twoDaysAgo]; | ||
| const uniqueDates = [...new Set(activities.map(date => date.toDateString()))]; | ||
|
|
||
| expect(uniqueDates.length).toBe(3); | ||
| }); | ||
|
|
||
| it('should handle broken streaks correctly', () => { | ||
| const today = new Date(); | ||
| const threeDaysAgo = new Date(today); | ||
| threeDaysAgo.setDate(threeDaysAgo.getDate() - 3); | ||
| const fourDaysAgo = new Date(today); | ||
| fourDaysAgo.setDate(fourDaysAgo.getDate() - 4); | ||
|
|
||
| const activities = [today, threeDaysAgo, fourDaysAgo]; | ||
| const uniqueDates = [...new Set(activities.map(date => date.toDateString()))]; | ||
|
|
||
| expect(uniqueDates.length).toBe(3); | ||
| }); | ||
|
|
||
| it('should calculate weekly consistency correctly', () => { | ||
| const oneWeekAgo = new Date(); | ||
| oneWeekAgo.setDate(oneWeekAgo.getDate() - 7); | ||
|
|
||
| // Create activities for 4 different days in the last week | ||
| const activities = [ | ||
| { date: new Date(oneWeekAgo.getTime() + 1 * 24 * 60 * 60 * 1000) }, | ||
| { date: new Date(oneWeekAgo.getTime() + 2 * 24 * 60 * 60 * 1000) }, | ||
| { date: new Date(oneWeekAgo.getTime() + 3 * 24 * 60 * 60 * 1000) }, | ||
| { date: new Date(oneWeekAgo.getTime() + 4 * 24 * 60 * 60 * 1000) }, | ||
| ]; | ||
|
|
||
| const uniqueDays = new Set( | ||
| activities.map(activity => activity.date.toDateString()) | ||
| ).size; | ||
|
|
||
| expect(uniqueDays).toBe(4); | ||
| expect(uniqueDays >= 4).toBe(true); // Should qualify for consistency achievement | ||
| }); | ||
| }); | ||
|
|
||
| describe('Achievement Definitions', () => { | ||
| it('should have proper achievement metadata structure', () => { | ||
| const mockAchievement = { | ||
| id: 'test-achievement', | ||
| name: 'Streak Starter', | ||
| description: 'Work out for 3 days in a row', | ||
| icon: '🔥', | ||
| count: 1, | ||
| color: '#ff6b35', | ||
| gradient: 'linear-gradient(135deg, #ff6b35, #f7931e)', | ||
| earnedAt: new Date() | ||
| }; | ||
|
|
||
| expect(mockAchievement.name).toBe('Streak Starter'); | ||
| expect(mockAchievement.description).toBe('Work out for 3 days in a row'); | ||
| expect(mockAchievement.icon).toBe('🔥'); | ||
| expect(mockAchievement.count).toBe(1); | ||
| expect(mockAchievement.color).toBe('#ff6b35'); | ||
| expect(mockAchievement.gradient).toContain('linear-gradient'); | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The test file has minimal coverage and doesn't test the actual database operations or integration with PrismaClient. The tests are mostly checking constant values. Consider adding integration tests that mock PrismaClient to test: 1) Achievement creation flow, 2) Streak calculation with real date scenarios, 3) Edge cases (timezone issues, leap years, etc.), and 4) Concurrent achievement awards.
Severity: MEDIUM
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: app/src/tests/AchievementSystem.test.tsx#L1-L87
Potential issue: The test file has minimal coverage and doesn't test the actual database
operations or integration with PrismaClient. The tests are mostly checking constant
values. Consider adding integration tests that mock PrismaClient to test: 1) Achievement
creation flow, 2) Streak calculation with real date scenarios, 3) Edge cases (timezone
issues, leap years, etc.), and 4) Concurrent achievement awards.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 7293328
Comment on lines
+15
to
+28
| }); | ||
|
|
||
| describe('Achievement Logic', () => { | ||
| it('should calculate streak correctly for consecutive days', () => { | ||
| const today = new Date(); | ||
| const yesterday = new Date(today); | ||
| yesterday.setDate(yesterday.getDate() - 1); | ||
| const twoDaysAgo = new Date(today); | ||
| twoDaysAgo.setDate(twoDaysAgo.getDate() - 2); | ||
|
|
||
| // Test that we can create date arrays for streak calculation | ||
| const activities = [today, yesterday, twoDaysAgo]; | ||
| const uniqueDates = [...new Set(activities.map(date => date.toDateString()))]; | ||
|
|
There was a problem hiding this comment.
The streak calculation test at line 17-28 doesn't properly validate the calculation logic. It only checks that dates can be placed in an array, not that the streak calculation algorithm works correctly. Add tests with real streak scenarios (e.g., 3-day streak, broken streaks, current vs. longest streaks) and verify the AchievementService methods return expected values.
Severity: MEDIUM
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: app/src/tests/AchievementSystem.test.tsx#L15-L28
Potential issue: The streak calculation test at line 17-28 doesn't properly validate the
calculation logic. It only checks that dates can be placed in an array, not that the
streak calculation algorithm works correctly. Add tests with real streak scenarios
(e.g., 3-day streak, broken streaks, current vs. longest streaks) and verify the
AchievementService methods return expected values.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 7293328
Comment on lines
+133
to
+139
| // Increment count | ||
| const updatedAchievement = await prisma.achievement.update({ | ||
| where: { | ||
| id: existingAchievement.id | ||
| }, | ||
| data: { | ||
| count: existingAchievement.count + 1, |
There was a problem hiding this comment.
The POST endpoint updates the earnedAt timestamp every time an achievement is re-earned. This changes the 'earned date' to the current time rather than keeping the original earned date. Consider either: 1) Not updating earnedAt on re-awards, or 2) Adding a separate lastAwardedAt field to track when it was most recently awarded. The current behavior could be confusing to users.
Severity: MEDIUM
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: app/src/app/api/achievements/route.ts#L133-L139
Potential issue: The POST endpoint updates the `earnedAt` timestamp every time an
achievement is re-earned. This changes the 'earned date' to the current time rather than
keeping the original earned date. Consider either: 1) Not updating earnedAt on
re-awards, or 2) Adding a separate `lastAwardedAt` field to track when it was most
recently awarded. The current behavior could be confusing to users.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 7293328
Comment on lines
+110
to
+135
| * Check and award consistency achievements (4+ days in a week) | ||
| */ | ||
| static async checkConsistencyAchievements(userId: string): Promise<void> { | ||
| const oneWeekAgo = new Date(); | ||
| oneWeekAgo.setDate(oneWeekAgo.getDate() - 7); | ||
|
|
||
| // Get activities from the last week | ||
| const weeklyActivities = await prisma.activity.findMany({ | ||
| where: { | ||
| userId, | ||
| date: { | ||
| gte: oneWeekAgo | ||
| } | ||
| }, | ||
| select: { date: true } | ||
| }); | ||
|
|
||
| // Count unique days with activities | ||
| const uniqueDays = new Set( | ||
| weeklyActivities.map(activity => activity.date.toDateString()) | ||
| ).size; | ||
|
|
||
| // Award consistency achievement if 4+ days | ||
| if (uniqueDays >= 4) { | ||
| await this.awardAchievement(userId, AchievementType.CONSISTENCY_CHAMPION); | ||
| } |
There was a problem hiding this comment.
The checkConsistencyAchievements method calculates weekly consistency but only awards the achievement once per check. If called multiple times in a week when the user has 4+ activity days, it will increment the count. However, the achievement definition suggests it should only be earned once per week. Consider adding logic to prevent re-awarding within the same week or clarifying the intended behavior.
Severity: MEDIUM
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: app/src/lib/achievementService.ts#L110-L135
Potential issue: The checkConsistencyAchievements method calculates weekly consistency
but only awards the achievement once per check. If called multiple times in a week when
the user has 4+ activity days, it will increment the count. However, the achievement
definition suggests it should only be earned once per week. Consider adding logic to
prevent re-awarding within the same week or clarifying the intended behavior.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 7293328
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.
Achievements for the UX - added to the home page