Add Zod models, types, UI updates, and tests for leaderboards and users#8
Add Zod models, types, UI updates, and tests for leaderboards and users#8
Conversation
WalkthroughThe changes introduce placeholder UI for empty private leaderboards, error boundaries for the daily puzzle screen, and comprehensive unit tests for leaderboard and puzzle components. Leaderboard score map logic is adjusted to ensure the current user is always included. Additional image assets are preloaded, and new test fixtures are provided for users and leaderboards. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DailyPuzzleScreen
participant PostHog
participant Sentry
participant ErrorBoundary
User->>DailyPuzzleScreen: Open daily puzzle screen
DailyPuzzleScreen->>DailyPuzzleScreen: Fetch puzzle data
alt Puzzle data missing and not loading
DailyPuzzleScreen->>PostHog: capture error
DailyPuzzleScreen->>Sentry: capture error
DailyPuzzleScreen->>ErrorBoundary: throw error
ErrorBoundary->>User: Show error UI with retry
else Puzzle data loading
DailyPuzzleScreen->>User: Show loading state
else Puzzle data present
DailyPuzzleScreen->>User: Show puzzle UI
end
sequenceDiagram
participant User
participant LeaderboardScreen
User->>LeaderboardScreen: Open leaderboard screen
LeaderboardScreen->>LeaderboardScreen: Fetch private leaderboards
alt Private leaderboards exist
LeaderboardScreen->>User: Show private leaderboard cards
else No private leaderboards
LeaderboardScreen->>User: Show placeholder image and message
end
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
✨ Finishing Touches
🪧 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 (
|
|
🚀 Expo preview is ready!
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/__tests__/leaderboards/weekly-leaderboard.test.tsx (3)
8-8: Fix the typo in the imported test fixture name.The import name
testPriverLeaderboard1appears to be a typo. It should likely betestPrivateLeaderboard1to match the semantic meaning of a private leaderboard.-import { testGlobalLeaderboard1, testPriverLeaderboard1 } from '@/tests/fixtures/leaderboards'; +import { testGlobalLeaderboard1, testPrivateLeaderboard1 } from '@/tests/fixtures/leaderboards';
40-40: Update variable references to match the corrected fixture name.These references should be updated to use the corrected fixture name for consistency.
- leaderboards: [testPriverLeaderboard1], + leaderboards: [testPrivateLeaderboard1],- expect(screen.queryByText(testPriverLeaderboard1.name!)).toBeOnTheScreen(); + expect(screen.queryByText(testPrivateLeaderboard1.name!)).toBeOnTheScreen();- expect(mockPresentLeaderboardActions).toHaveBeenCalledWith(testPriverLeaderboard1); + expect(mockPresentLeaderboardActions).toHaveBeenCalledWith(testPrivateLeaderboard1);Also applies to: 71-71, 92-92
71-71: Consider safer handling of potentially null name property.The non-null assertion
!assumes the name property will always exist, but leaderboards might have null names. Consider using optional chaining or a fallback value for more robust testing.- expect(screen.queryByText(testPrivateLeaderboard1.name!)).toBeOnTheScreen(); + expect(screen.queryByText(testPrivateLeaderboard1.name ?? testPrivateLeaderboard1._id)).toBeOnTheScreen();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/assets/images/error.pngis excluded by!**/*.pngsrc/assets/images/no-leaderboards.pngis excluded by!**/*.png
📒 Files selected for processing (12)
convex/leaderboards/models.ts(1 hunks)convex/leaderboards/queries.ts(2 hunks)src/__tests__/leaderboards/all-time-leaderboard.test.tsx(1 hunks)src/__tests__/leaderboards/weekly-leaderboard.test.tsx(1 hunks)src/__tests__/play/daily-puzzle.test.tsx(1 hunks)src/app/(authenticated)/leaderboards/all-time-leaderboard.tsx(3 hunks)src/app/(authenticated)/leaderboards/weekly-leaderboard.tsx(3 hunks)src/app/(authenticated)/play/daily-puzzle.tsx(3 hunks)src/app/_layout.tsx(1 hunks)src/components/ui/Card/Card.tsx(1 hunks)src/tests/fixtures/leaderboards.ts(1 hunks)src/tests/fixtures/users.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: zigcccc
PR: zigcccc/petka-app#7
File: src/hooks/useUser/useUser.test.ts:183-186
Timestamp: 2025-07-07T06:21:28.647Z
Learning: The user (zigcccc) is comfortable with using `@ts-expect-error` TypeScript error suppression in test code, considering it acceptable for test data even when it compromises type safety.
Learnt from: zigcccc
PR: zigcccc/petka-app#7
File: src/hooks/useTrainingPuzzle/useTrainingPuzzle.ts:48-51
Timestamp: 2025-07-07T06:22:14.787Z
Learning: In src/hooks/useTrainingPuzzle/useTrainingPuzzle.ts and similar mutation calls, the user (zigcccc) prefers to rely on server-side validation rather than adding client-side validation checks for puzzle and user data before calling mutations - the server will reject invalid requests.
Learnt from: zigcccc
PR: zigcccc/petka-app#7
File: src/hooks/useTrainingPuzzle/useTrainingPuzzle.ts:31-37
Timestamp: 2025-07-07T06:21:55.962Z
Learning: In src/hooks/useTrainingPuzzle/useTrainingPuzzle.ts, the user (zigcccc) prefers to rely on server-side validation rather than adding client-side validation checks before using non-null assertions. They are comfortable with letting the server reject requests if required data is missing.
Learnt from: zigcccc
PR: zigcccc/petka-app#7
File: src/app/(authenticated)/play/daily-puzzle-solved.tsx:66-79
Timestamp: 2025-07-07T06:08:49.592Z
Learning: In src/app/(authenticated)/play/daily-puzzle-solved.tsx, the user is fine with the iOS-only ActionSheetIOS sharing implementation for now, treating it as a work-in-progress item that may be addressed later for cross-platform compatibility.
src/components/ui/Card/Card.tsx (1)
Learnt from: zigcccc
PR: zigcccc/petka-app#7
File: src/app/(authenticated)/play/daily-puzzle-solved.tsx:66-79
Timestamp: 2025-07-07T06:08:49.592Z
Learning: In src/app/(authenticated)/play/daily-puzzle-solved.tsx, the user is fine with the iOS-only ActionSheetIOS sharing implementation for now, treating it as a work-in-progress item that may be addressed later for cross-platform compatibility.
src/app/_layout.tsx (1)
Learnt from: zigcccc
PR: zigcccc/petka-app#7
File: src/app/(authenticated)/play/daily-puzzle-solved.tsx:66-79
Timestamp: 2025-07-07T06:08:49.592Z
Learning: In src/app/(authenticated)/play/daily-puzzle-solved.tsx, the user is fine with the iOS-only ActionSheetIOS sharing implementation for now, treating it as a work-in-progress item that may be addressed later for cross-platform compatibility.
src/tests/fixtures/users.ts (2)
Learnt from: zigcccc
PR: zigcccc/petka-app#7
File: src/hooks/useUser/useUser.test.ts:183-186
Timestamp: 2025-07-07T06:21:28.647Z
Learning: The user (zigcccc) is comfortable with using `@ts-expect-error` TypeScript error suppression in test code, considering it acceptable for test data even when it compromises type safety.
Learnt from: zigcccc
PR: zigcccc/petka-app#7
File: src/hooks/useTrainingPuzzle/useTrainingPuzzle.ts:48-51
Timestamp: 2025-07-07T06:22:14.787Z
Learning: In src/hooks/useTrainingPuzzle/useTrainingPuzzle.ts and similar mutation calls, the user (zigcccc) prefers to rely on server-side validation rather than adding client-side validation checks for puzzle and user data before calling mutations - the server will reject invalid requests.
src/app/(authenticated)/leaderboards/weekly-leaderboard.tsx (1)
Learnt from: zigcccc
PR: zigcccc/petka-app#7
File: src/app/(authenticated)/play/daily-puzzle-solved.tsx:66-79
Timestamp: 2025-07-07T06:08:49.592Z
Learning: In src/app/(authenticated)/play/daily-puzzle-solved.tsx, the user is fine with the iOS-only ActionSheetIOS sharing implementation for now, treating it as a work-in-progress item that may be addressed later for cross-platform compatibility.
src/app/(authenticated)/leaderboards/all-time-leaderboard.tsx (1)
Learnt from: zigcccc
PR: zigcccc/petka-app#7
File: src/app/(authenticated)/play/daily-puzzle-solved.tsx:66-79
Timestamp: 2025-07-07T06:08:49.592Z
Learning: In src/app/(authenticated)/play/daily-puzzle-solved.tsx, the user is fine with the iOS-only ActionSheetIOS sharing implementation for now, treating it as a work-in-progress item that may be addressed later for cross-platform compatibility.
src/app/(authenticated)/play/daily-puzzle.tsx (6)
Learnt from: zigcccc
PR: zigcccc/petka-app#7
File: src/app/(authenticated)/play/daily-puzzle-solved.tsx:66-79
Timestamp: 2025-07-07T06:08:49.592Z
Learning: In src/app/(authenticated)/play/daily-puzzle-solved.tsx, the user is fine with the iOS-only ActionSheetIOS sharing implementation for now, treating it as a work-in-progress item that may be addressed later for cross-platform compatibility.
Learnt from: zigcccc
PR: zigcccc/petka-app#7
File: src/app/(authenticated)/play/training-puzzle-solved.tsx:19-26
Timestamp: 2025-07-07T06:09:21.606Z
Learning: In the TrainingPuzzleSolvedScreen component (src/app/(authenticated)/play/training-puzzle-solved.tsx), the onMarkAsSolved() call should NOT be awaited - this is intentional "fire and forget" behavior where navigation happens immediately without waiting for the marking operation to complete.
Learnt from: zigcccc
PR: zigcccc/petka-app#7
File: src/hooks/useUser/useUser.ts:55-62
Timestamp: 2025-07-07T06:24:56.001Z
Learning: In src/hooks/useUser/useUser.ts, the handleUpdateUser function intentionally does not include try-catch error handling because errors are meant to bubble up to the calling form components. This is by design - the user (zigcccc) prefers to have form-level error handling rather than utility function-level error handling for update operations.
Learnt from: zigcccc
PR: zigcccc/petka-app#7
File: src/hooks/useTrainingPuzzle/useTrainingPuzzle.ts:48-51
Timestamp: 2025-07-07T06:22:14.787Z
Learning: In src/hooks/useTrainingPuzzle/useTrainingPuzzle.ts and similar mutation calls, the user (zigcccc) prefers to rely on server-side validation rather than adding client-side validation checks for puzzle and user data before calling mutations - the server will reject invalid requests.
Learnt from: zigcccc
PR: zigcccc/petka-app#7
File: src/hooks/useTrainingPuzzle/useTrainingPuzzle.ts:39-46
Timestamp: 2025-07-07T06:22:32.646Z
Learning: In the useTrainingPuzzle hook (src/hooks/useTrainingPuzzle/useTrainingPuzzle.ts), the user prefers server-side validation over client-side validation for user authentication/authorization checks. The server is expected to handle validation when user._id is null/undefined, so client-side validation is not needed.
Learnt from: zigcccc
PR: zigcccc/petka-app#7
File: src/hooks/useTrainingPuzzle/useTrainingPuzzle.ts:31-37
Timestamp: 2025-07-07T06:21:55.962Z
Learning: In src/hooks/useTrainingPuzzle/useTrainingPuzzle.ts, the user (zigcccc) prefers to rely on server-side validation rather than adding client-side validation checks before using non-null assertions. They are comfortable with letting the server reject requests if required data is missing.
src/__tests__/leaderboards/all-time-leaderboard.test.tsx (1)
Learnt from: zigcccc
PR: zigcccc/petka-app#7
File: src/app/(authenticated)/play/daily-puzzle-solved.tsx:66-79
Timestamp: 2025-07-07T06:08:49.592Z
Learning: In src/app/(authenticated)/play/daily-puzzle-solved.tsx, the user is fine with the iOS-only ActionSheetIOS sharing implementation for now, treating it as a work-in-progress item that may be addressed later for cross-platform compatibility.
src/__tests__/leaderboards/weekly-leaderboard.test.tsx (1)
Learnt from: zigcccc
PR: zigcccc/petka-app#7
File: src/app/(authenticated)/play/daily-puzzle-solved.tsx:66-79
Timestamp: 2025-07-07T06:08:49.592Z
Learning: In src/app/(authenticated)/play/daily-puzzle-solved.tsx, the user is fine with the iOS-only ActionSheetIOS sharing implementation for now, treating it as a work-in-progress item that may be addressed later for cross-platform compatibility.
src/__tests__/play/daily-puzzle.test.tsx (3)
Learnt from: zigcccc
PR: zigcccc/petka-app#7
File: src/app/(authenticated)/play/daily-puzzle-solved.tsx:66-79
Timestamp: 2025-07-07T06:08:49.592Z
Learning: In src/app/(authenticated)/play/daily-puzzle-solved.tsx, the user is fine with the iOS-only ActionSheetIOS sharing implementation for now, treating it as a work-in-progress item that may be addressed later for cross-platform compatibility.
Learnt from: zigcccc
PR: zigcccc/petka-app#7
File: src/app/(authenticated)/play/training-puzzle-solved.tsx:19-26
Timestamp: 2025-07-07T06:09:21.606Z
Learning: In the TrainingPuzzleSolvedScreen component (src/app/(authenticated)/play/training-puzzle-solved.tsx), the onMarkAsSolved() call should NOT be awaited - this is intentional "fire and forget" behavior where navigation happens immediately without waiting for the marking operation to complete.
Learnt from: zigcccc
PR: zigcccc/petka-app#7
File: src/hooks/useTrainingPuzzle/useTrainingPuzzle.ts:48-51
Timestamp: 2025-07-07T06:22:14.787Z
Learning: In src/hooks/useTrainingPuzzle/useTrainingPuzzle.ts and similar mutation calls, the user (zigcccc) prefers to rely on server-side validation rather than adding client-side validation checks for puzzle and user data before calling mutations - the server will reject invalid requests.
🧬 Code Graph Analysis (5)
convex/leaderboards/queries.ts (1)
convex/_generated/dataModel.d.ts (1)
Id(48-49)
src/tests/fixtures/users.ts (2)
convex/users/models.ts (1)
User(11-11)convex/_generated/dataModel.d.ts (1)
Id(48-49)
src/app/(authenticated)/play/daily-puzzle.tsx (2)
src/components/ui/Text/Text.tsx (1)
Text(13-35)src/components/ui/Button/Button.tsx (1)
Button(37-90)
src/tests/fixtures/leaderboards.ts (3)
convex/leaderboards/models.ts (2)
LeaderboardWithUserScores(45-45)leaderboardType(11-11)convex/_generated/dataModel.d.ts (1)
Id(48-49)src/tests/fixtures/users.ts (2)
testUser1(4-9)testUser2(11-16)
src/__tests__/leaderboards/weekly-leaderboard.test.tsx (5)
src/hooks/useGlobalLeaderboard/useGlobalLeaderboard.ts (1)
useGlobalLeaderboard(6-9)src/hooks/useLeaderboards/useLeaderboards.ts (1)
useLeaderboards(20-345)src/tests/fixtures/leaderboards.ts (2)
testGlobalLeaderboard1(6-15)testPriverLeaderboard1(17-29)src/app/(authenticated)/leaderboards/weekly-leaderboard.tsx (1)
WeeklyLeaderboardScreen(12-70)convex/leaderboards/models.ts (2)
leaderboardRange(8-8)leaderboardType(11-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Sonar analysis
- GitHub Check: Publish Expo Preview
🔇 Additional comments (20)
src/components/ui/Card/Card.tsx (1)
33-33: LGTM! Good testing practice.Adding testID for the action trigger improves component testability without affecting functionality.
src/app/_layout.tsx (1)
63-64: LGTM! Good performance optimization.Preloading the new image assets ensures they're cached and ready for use, improving user experience.
src/tests/fixtures/users.ts (1)
11-16: LGTM! Well-structured test fixture.The new test user follows the same pattern as existing fixtures and provides necessary test data for leaderboard functionality.
convex/leaderboards/queries.ts (2)
57-59: LGTM! Ensures current user presence in leaderboard.Initializing the score map with the current user's ID and score 0 ensures consistent behavior where the current user always appears in leaderboard results, even with no scores.
127-129: LGTM! Consistent with the list query improvement.Same beneficial change as in the
listfunction, ensuring the current user always appears in the global leaderboard results.src/app/(authenticated)/leaderboards/weekly-leaderboard.tsx (2)
39-58: LGTM! Clean conditional rendering with good UX.The conditional rendering for private leaderboards with a placeholder state provides excellent user experience when no private leaderboards exist. The implementation correctly handles both null and empty array cases.
93-94: Good spacing adjustment.The increased gap from
theme.spacing[6]totheme.spacing[8]improves visual hierarchy between leaderboard cards.src/app/(authenticated)/play/daily-puzzle.tsx (2)
12-27: Excellent error boundary implementation.The ErrorBoundary component provides a user-friendly error interface with proper retry functionality. The UI is well-structured with clear messaging in the local language.
52-58: Good error capture and monitoring.The error handling correctly captures the exception with both PostHog and Sentry before throwing to trigger the ErrorBoundary. This provides good observability for debugging production issues.
src/app/(authenticated)/leaderboards/all-time-leaderboard.tsx (1)
39-58: Consistent implementation with weekly leaderboard.The conditional rendering and placeholder UI implementation matches the weekly leaderboard screen perfectly, providing consistent user experience across both leaderboard types.
src/__tests__/leaderboards/all-time-leaderboard.test.tsx (2)
8-8: Update fixture import after fixing the typo.The import references
testPriverLeaderboard1which has a typo in the fixture file. Once the fixture is renamed totestPrivateLeaderboard1, this import will need to be updated.After fixing the typo in the fixture file, update this import:
-import { testGlobalLeaderboard1, testPriverLeaderboard1 } from '@/tests/fixtures/leaderboards'; +import { testGlobalLeaderboard1, testPrivateLeaderboard1 } from '@/tests/fixtures/leaderboards';And update all references throughout the test file.
26-104: Excellent comprehensive test coverage.The test suite thoroughly covers all aspects of the component:
- Hook integration and parameter passing
- Navigation title setting
- UI rendering for different states
- Button interactions and event handling
- Empty state handling with parameterized tests
The structure is clean and maintainable.
src/__tests__/leaderboards/weekly-leaderboard.test.tsx (1)
53-58: Excellent test coverage and structure.The test suite comprehensively covers:
- Hook parameter validation
- Navigation title setting
- UI rendering with mock data
- User interaction handling
- Empty state scenarios
The use of parameterized tests for empty states and proper mocking strategy demonstrates good testing practices.
Also applies to: 66-72, 74-85, 87-93, 95-103
src/__tests__/play/daily-puzzle.test.tsx (7)
1-28: Well-structured test setup with comprehensive mocking.The mock configuration properly preserves actual implementations while overriding specific functionality. The inclusion of Sentry and PostHog mocking demonstrates good integration testing practices.
41-47: Good use of default test data structure.The
defaultDailyPuzzleOptionsprovides a clean base configuration that can be easily overridden in individual tests, promoting test maintainability and readability.
60-67: Excellent navigation behavior testing.These tests properly verify the conditional navigation logic and header configuration based on the puzzle's solved state, ensuring the component behaves correctly in both scenarios.
Also applies to: 69-76
78-87: Robust error handling validation.The test correctly verifies that:
- An error is thrown when puzzle data is missing and not loading
- Both PostHog and Sentry capture the exception for proper error tracking
- The error handling integrates with the analytics and monitoring systems
89-99: Effective use of parameterized testing.The parameterized test elegantly covers multiple scenarios (loading vs. data available) while ensuring no false positive errors are triggered, maintaining clean test output.
114-115: Smart UI component testing with regex patterns.Using regex patterns for testId matching (
/^guess-grid--row-\d+$/) provides flexible validation that works regardless of the exact number of grid rows, making the test more maintainable.
119-131: Comprehensive ErrorBoundary testing.The ErrorBoundary tests properly verify both the error display and retry functionality, ensuring the error recovery mechanism works as expected.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Style
Chores