Skip to content

hide simulate leaderboard for non-admin#7941

Merged
smallst merged 1 commit intomasterfrom
yuqiang/remove-simulator-leaderboard-for-normal-users
Mar 14, 2025
Merged

hide simulate leaderboard for non-admin#7941
smallst merged 1 commit intomasterfrom
yuqiang/remove-simulator-leaderboard-for-normal-users

Conversation

@smallst
Copy link
Copy Markdown
Contributor

@smallst smallst commented Mar 14, 2025

result:
image

acutally i notice that course-ladder doesn't show up the simulate leaderboard but actually load it. that's why we have so many hits I think. and also notice that most of hero-ladders are closed, so most of home user may already turn to ai-league. i think hide this leaderboard make sense. let's see if anyone complain about it.

fix ENG-1679

Summary by CodeRabbit

  • New Features
    • Refined the ladder view so that simulation statistics and the "Simulate" tab are displayed based on user permissions. Non-admin users see a streamlined view while simulation details and controls are available only to admins.
    • Adjusted the data fetching for simulation metrics to ensure that leaderboard data is retrieved exclusively for authorized users.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 14, 2025

Walkthrough

This pull request refines the conditional logic used to control the visibility of simulation-related elements in the ladder view. In the Pug template, the rendering of the inline simulation stats and the "Simulate" tab link now depends on whether the user is an admin and, in one case, whether the league type is not 'course'. Similarly, the JavaScript fetch method in the SimulateTabView has been updated to include an admin check alongside its ladder type condition, thereby restricting data fetch operations to admin users only.

Changes

File(s) Change Summary
app/templates/.../ladder.pug
app/views/ladder/SimulateTabView.js
Introduced admin-based conditional logic. In the Pug template, simulation stats and the "Simulate" tab link are rendered only if the user is an admin (and for the tab link, the league type is not 'course'). In the JS file, the fetch method now checks both that the level is not a ladder type and that the user is an admin before proceeding.

Sequence Diagram(s)

sequenceDiagram
    participant U as User
    participant T as Ladder Template
    participant F as SimulateTabView.fetch
    U->>T: Load Ladder Page
    alt User is Non-Admin
        T->>T: Skip rendering simulation stats and "Simulate" tab
        Note over T,U: Simulation elements remain hidden
    else User is Admin
        T->>T: Render simulation stats and "Simulate" tab (if leagueType ≠ 'course')
    end
    U->>F: Trigger data fetch (if applicable)
    alt Admin & Level not ladder
        F->>F: Execute fetch logic for leaderboard data
    else
        F->>F: Skip fetching data
    end
Loading

Possibly related PRs

Suggested reviewers

  • adamkecskes
  • mrfinch

Poem

In a field of code I hop,
Admin checks at every stop.
Stats and tabs now bloom for those
Who hold the power – everyone knows!
ASCII carrots guide my way, 🥕
Celebrating changes in a joyful play!

— A code-loving rabbit!

Tip

⚡🧪 Multi-step agentic review comment chat (experimental)
  • We're introducing multi-step agentic chat in review comments. This experimental feature enhances review discussions with the CodeRabbit agentic chat by enabling advanced interactions, including the ability to create pull requests directly from comments.
    - To enable this feature, set early_access to true under in the settings.

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e7f2f18 and 2b9742f.

📒 Files selected for processing (2)
  • app/templates/play/ladder/ladder.pug (2 hunks)
  • app/views/ladder/SimulateTabView.js (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Node.js CI (18.10.0)
🔇 Additional comments (4)
app/templates/play/ladder/ladder.pug (3)

215-217: LGTM! The Simulate tab link is now admin-only.

This change correctly restricts the Simulate tab link to be visible only for admin users when the league type is not 'course'.


291-293: LGTM! Simulations tab content is properly restricted.

This change correctly ensures that the simulation tab content is only rendered for admin users, which aligns with the PR objectives to hide the simulate leaderboard from non-admin users.


206-208:

Details

❓ Verification inconclusive

Confirmation needed: Is restricting the simulation stats for non-admins correct?

The conditional logic added here shows the inline simulation stats and simulate-tab-view when the user is NOT an admin (if !me.isAdmin()). This seems to contradict the PR objective of hiding the simulate leaderboard for non-admin users.

Should this condition be inverted to if me.isAdmin() to ensure only admins see these elements?


🏁 Script executed:

#!/bin/bash
# Check how the simulate-tab-view is being used elsewhere in the codebase
# to confirm the intended visibility pattern

echo "Checking for other instances of simulate-tab-view and its visibility conditions:"
rg -A 5 -B 5 "simulate-tab-view" --type pug

Length of output: 242


Action Required: Confirm Conditional Rendering for Simulation Stats

The current code renders the simulation stats and simulate-tab-view when the user is not an admin (if !me.isAdmin()), but the PR’s objective is to hide the simulate leaderboard for non-admin users. To meet the PR objective, these elements should be visible only to admin users—i.e., the condition should be inverted to if me.isAdmin().

Since our initial search for other occurrences of simulate-tab-view was hindered by file-type recognition issues, please verify across the codebase that no other parts of the application rely on the current condition for proper behavior.

  • Action: Change the condition in app/templates/play/ladder/ladder.pug from if !me.isAdmin() to if me.isAdmin().
  • Next Step: Manually verify the impact of this change on the simulate leaderboard visibility across the app.
app/views/ladder/SimulateTabView.js (1)

181-195: LGTM! Data fetching is now properly restricted to admins.

This change ensures that the simulator leaderboard data is only fetched when the user is an admin, preventing unnecessary API calls for non-admin users. This directly addresses the PR objective of reducing unnecessary leaderboard hits.

✨ Finishing Touches
  • 📝 Generate Docstrings

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@smallst smallst merged commit 0a18983 into master Mar 14, 2025
3 checks passed
@smallst smallst deleted the yuqiang/remove-simulator-leaderboard-for-normal-users branch March 14, 2025 05:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants