794: Refactor Leaderboard

[alfardil]

794

Description of changes

Refactored to use Optional for Leaderboard types

Checklist before review

• I have done a thorough self-review of the PR
• Copilot has reviewed my latest changes, and all comments have been fixed and/or closed.
• If I have made database changes, I have made sure I followed all the db repo rules listed in the wiki here. (check if no db changes)
• All tests have passed
• I have successfully deployed this PR to staging
• I have done manual QA in both dev (and staging if possible) and attached screenshots below.

Screenshots

Dev

All tests passed in dev

Staging

[github-actions]

Available PR Commands

• /ai - Triggers all AI review commands at once
• /review - AI review of the PR changes
• /describe - AI-powered description of the PR
• /improve - AI-powered suggestions
• /deploy - Deploy to staging

See: https://github.com/tahminator/codebloom/wiki/CI-Commands

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Unsafe Optional.get() Several endpoints call .get() on Optionals from repository/manager without presence checks, which can throw and return 500 instead of a 404/empty response. Please guard with isPresent()/orElseThrow(ResponseStatusException) and update the null-checks accordingly. Optional<Leaderboard> leaderboardData = leaderboardManager.getLeaderboardMetadata( leaderboardRepository.getRecentLeaderboardMetadata().get().getId()); return ResponseEntity.ok() .body(ApiResponder.success( "All leaderboards found!", LeaderboardDto.fromLeaderboard(leaderboardData.get()))); Unsafe Optional.get() Discord notifications build paths/titles using currentLeaderboard.get() with no fallback if no active leaderboard exists. Add presence checks (and a graceful no-leaderboard behavior) to avoid NoSuchElementException in production jobs. topUsersSection, club.getName(), serverUrlUtils.getUrl(), currentLeaderboard.get().getId(), club.getTag().name().toLowerCase(), Optional Handling Recent leaderboard is now Optional but code still assumes non-null and calls .get() directly. Replace the null check with isEmpty() and handle absence explicitly before dereferencing. boolean isTooLate = recentLeaderboard.get().getCreatedAt().isAfter(leetcodeSubmission.getTimestamp());

[tahminator]

/ai

[tahminator]

/review

[tahminator]

/describe

[tahminator]

/improve

[github-actions]

Preparing PR description...

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Possible Issue Creating a new leaderboard now throws when there is no current leaderboard (using Optional.isEmpty → RuntimeException), which changes prior behavior and conflicts with tests expecting success when none exists. Treat absence as “no previous leaderboard” and proceed instead of throwing. // BE VERY CAREFUL WITH THIS ROUTE. IT WILL DEACTIVATE THE PREVIOUS LEADERBOARD // (however, it should be in a recoverable state, as it just gets toggled to be // deactivated, not deleted). Optional<Leaderboard> currentLeaderboard = leaderboardRepository.getRecentLeaderboardMetadata(); if (currentLeaderboard.isEmpty()) { throw new RuntimeException("No recent leaderboard found."); } discordClubManager.sendLeaderboardCompletedDiscordMessageToAllClubs(); leaderboardManager.generateAchievementsForAllWinners(); leaderboardRepository.disableLeaderboardById(currentLeaderboard.get().getId()); Optional Misuse Unsafe Optional.get() usage on the current leaderboard can cause 500s when none exists (e.g., fetching current metadata and users). Check presence and return 404 or a typed error instead of calling get() directly; similarly avoid Optional.get() on ranked-user lookups and handle empty results. final HttpServletRequest request) { FakeLag.sleep(650); Optional<Leaderboard> leaderboardData = leaderboardManager.getLeaderboardMetadata( leaderboardRepository.getRecentLeaderboardMetadata().get().getId()); if (leaderboardData.isEmpty()) { throw new ResponseStatusException(HttpStatus.NOT_FOUND, "Leaderboard cannot be found or does not exist."); } return ResponseEntity.ok() .body(ApiResponder.success( "All leaderboards found!", LeaderboardDto.fromLeaderboard(leaderboardData.get()))); } Null Optional Risk Optional fields lack default values, so builder-created instances may have null Optionals, causing NPEs where .orElse(...) is called (e.g., DTO mapping). Consider @Builder.Default with Optional.empty() or enforce non-null Optionals at construction. private LocalDateTime createdAt; @NullColumn private Optional<LocalDateTime> deletedAt; @NullColumn private Optional<LocalDateTime> shouldExpireBy; @NullColumn private Optional<String> syntaxHighlightingLanguage; }

[alfardil]

/ai

[alfardil]

/review

[alfardil]

/describe

[alfardil]

/improve

[tahminator]

/review

[tahminator]

/describe

[tahminator]

/improve

[github-actions]

Preparing PR description...

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Optional Defaults Optional fields on the model can be null if not explicitly set, which will cause NPEs when calling methods like .orElse(...) or .isPresent(). Consider initializing Optional fields to Optional.empty() (e.g., with @Builder.Default) and remove null checks elsewhere for consistency. private Optional<LocalDateTime> deletedAt; @NullColumn private Optional<LocalDateTime> shouldExpireBy; @NullColumn private Optional<String> syntaxHighlightingLanguage; } Test Assertion Bug Tests use assertNotNull(optional) on Optional results, which always passes even when empty. Replace with assertTrue(optional.isPresent()) (and adjust subsequent .get() calls) to avoid false positives and potential NoSuchElementException. assertNotNull(patinaResult, "User with Patina tag should appear in Patina filter"); assertNotNull(baruchResult, "User with Baruch tag should appear in Baruch filter"); assertEquals( patinaResult.get().getItem(), baruchResult.get().getItem(), "Same user should be returned in both filters"); } Error Handling Throwing RuntimeException when no active leaderboard is found (without logging) may crash background flows. Prefer logging with context and returning early, or using a controlled error path to avoid noisy failures in scheduled/notification workflows. Optional<Leaderboard> currentLeaderboard = leaderboardRepository.getRecentLeaderboardMetadata(); if (currentLeaderboard.isEmpty()) { throw new RuntimeException("No recent leaderboard found."); }

[alfardil]

/deploy

bug