Skip to content

Bugfix/leaderboard try again#4501

Open
blontd6 wants to merge 1 commit into
openfrontio:mainfrom
blontd6:bugfix/leaderboard-try-again
Open

Bugfix/leaderboard try again#4501
blontd6 wants to merge 1 commit into
openfrontio:mainfrom
blontd6:bugfix/leaderboard-try-again

Conversation

@blontd6

@blontd6 blontd6 commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Resolves #4500

Description:

Fixes a pagination bug on the 1v1 Ranked leaderboard modal where scrolling near the bottom of the first page triggers an infinite fetch loop/error and permanently displays a "Try Again" button in the footer.

What was changed:

  • Modified fetchPlayerLeaderboard in src/client/Api.ts to check if a failed response (!res.ok) is a 400 Bad Request containing a validation error about the page bounds.
  • If so, it returns "reached_limit", signaling the infinite-scroll component that it has successfully hit the end of the leaderboard pages, which stops further fetching and hides the error footer.

Please complete the following:

  • I have added screenshots for all UI updates
  • I process any text displayed to the user through translateText() and I've added it to the en.json file
  • I have added relevant tests to the test directory

Please put your Discord username so you can be contacted if a bug or regression is found:

blontd6

BEFORE:
Screenshot 2026-07-03 at 9 41 58 PM
AFTER:
Screenshot 2026-07-03 at 9 41 40 PM

@coderabbitai

coderabbitai Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

The fetchPlayerLeaderboard function now checks for HTTP 400 responses, parses the JSON body, and returns "reached_limit" when the error message references pagination limits, instead of falling through to generic error handling.

Changes

Leaderboard 400 Handling

Layer / File(s) Summary
Detect page-limit 400 response
src/client/Api.ts
Adds a branch for res.status === 400 that parses the response JSON, checks the message/error text for "page", and returns "reached_limit" on match; falls back to existing non-OK handling otherwise.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Poem

A bunny hopped to page one hundred and one,
Found a wall marked "400," thought all hope was done.
But now the code peeks at the words inside,
Sees "page" in the message, no more need to hide.
"Reached the limit!" it cheers, footer calm and clean —
No more angry buttons on the leaderboard scene. 🐇✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The change handles the reported 400 page-bounds error and returns reached_limit as required by #4500.
Out of Scope Changes check ✅ Passed The PR stays focused on the leaderboard pagination fix and does not add unrelated changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title is related to the leaderboard bug fix, though it is a bit terse.
Description check ✅ Passed The description matches the code change and explains the pagination bug fix clearly.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the small-fix Small fix (≤ 50 lines) — auto-applied by PR gate label Jul 4, 2026
@blontd6 blontd6 changed the title 2Bugfix/leaderboard try again Bugfix/leaderboard try again Jul 4, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/Api.ts`:
- Around line 393-404: The 400-handling in Api.ts is too broad because the
current page-related check can incorrectly map unrelated errors to
"reached_limit" and duplicates the stricter server-message check elsewhere.
Update the logic in Api.ts to use one shared helper for the reached-limit
detection and make it match the exact server text used by the existing branch
(for example, the "Page must be between" pattern) so only the intended
end-of-list response is swallowed and other 400s still surface normally.
🪄 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: 0c22aea8-cdaf-4d82-96b3-b4ae69514e70

📥 Commits

Reviewing files that changed from the base of the PR and between b0f85c5 and 1046aa3.

📒 Files selected for processing (1)
  • src/client/Api.ts

Comment thread src/client/Api.ts
@github-project-automation github-project-automation Bot moved this from Triage to Development in OpenFront Release Management Jul 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

small-fix Small fix (≤ 50 lines) — auto-applied by PR gate

Projects

Status: Development

Development

Successfully merging this pull request may close these issues.

Leaderboard "Try Again" button in footer

1 participant