-
-
Notifications
You must be signed in to change notification settings - Fork 323
feat: add github comment leaderboard #5363
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: add github comment leaderboard #5363
Conversation
…gic, added webhook and test
|
👋 Hi @mdkaifansari04! This pull request needs a peer review before it can be merged. Please request a review from a team member who is not:
Once a valid peer review is submitted, this check will pass automatically. Thank you! |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a GitHubComment model and migration, webhook handlers for GitHub comment events (issue_comment and review_comment), tests for webhook behavior, leaderboard aggregation for recent PR comments, and template updates to display a GitHub Comment Leaderboard. Changes
Sequence DiagramsequenceDiagram
participant GitHub
participant WebhookHandler as Webhook Handler
participant DB as Database
participant LeaderboardView as Leaderboard View
participant Template
GitHub->>WebhookHandler: POST "issue_comment" / "pull_request_review_comment"
activate WebhookHandler
WebhookHandler->>WebhookHandler: Validate payload (is PR?, not bot)
alt valid PR comment & not bot
WebhookHandler->>DB: Find GitHubIssue by repo+number
WebhookHandler->>DB: Upsert GitHubComment (comment_id, type, body, url, timestamps, fks)
else skip (non-PR or bot)
WebhookHandler-->>GitHub: 200 OK (no-op)
end
deactivate WebhookHandler
LeaderboardView->>DB: Query GitHubComment (last 6 months, filters)
activate LeaderboardView
DB-->>LeaderboardView: Comments grouped by user (count, latest)
LeaderboardView->>LeaderboardView: Aggregate & sort (count DESC, recent DESC)
LeaderboardView->>Template: Provide comment_leaderboard context
deactivate LeaderboardView
Template->>Template: Render GitHub Comment Leaderboard card
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro Knowledge base: Disabled due to 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code graph analysis (2)website/tests/test_webhook_comments.py (1)
website/views/user.py (1)
⏰ 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). (7)
🔇 Additional comments (4)
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. Comment |
📊 Monthly LeaderboardHi @mdkaifansari04! Here's how you rank for December 2025:
Leaderboard based on contributions in December 2025. Keep up the great work! 🚀 |
❌ Pre-commit checks failedThe pre-commit hooks found issues that need to be fixed. Please run the following commands locally to fix them: # Install pre-commit if you haven't already
pip install pre-commit
# Run pre-commit on all files
pre-commit run --all-files
# Or run pre-commit on staged files only
pre-commit runAfter running these commands, the pre-commit hooks will automatically fix most issues. 💡 Tip: You can set up pre-commit to run automatically on every commit by running: pre-commit installPre-commit outputFor more information, see the pre-commit documentation. |
| comment_id=comment_data.get("id"), | ||
| defaults={ | ||
| "github_issue": github_issue, | ||
| "user_profile": user_profile, | ||
| "body": comment_data.get("body", ""), | ||
| "comment_type": "issue_comment", | ||
| "created_at": dateutil_parser.parse(comment_data.get("created_at")), | ||
| "updated_at": dateutil_parser.parse(comment_data.get("updated_at")), | ||
| "url": comment_data.get("html_url"), | ||
| }, | ||
| ) |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds a GitHub PR comment leaderboard to the global leaderboard page, displaying top contributors who actively comment on pull requests across BLT repositories. The implementation captures PR comments via GitHub webhooks and displays the top 10 commenters from the last 6 months.
Key Changes
- New
GitHubCommentmodel to store PR comments with relationships to issues, users, and contributors - Two new webhook handlers (
handle_issue_comment_eventandhandle_review_comment_event) to process GitHub comment events - Comment leaderboard query and UI component displaying contributor names, avatars, and comment counts
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| website/models.py | Adds GitHubComment model with fields for storing comment data, relationships to issues/users/contributors, and metadata |
| website/migrations/0261_githubcomment.py | Database migration creating the GitHubComment table with appropriate fields and foreign keys |
| website/views/user.py | Implements webhook handlers for issue comments and review comments; adds comment leaderboard query aggregating comments by contributor |
| website/templates/leaderboard_global.html | Adds new comment leaderboard UI card; updates grid layout to accommodate new leaderboard section |
| website/tests/test_webhook_comments.py | Adds tests for issue comment webhook handler covering PR comments, non-PR comments, and bot filtering |
Comments suppressed due to low confidence (5)
website/views/user.py:1497
- The contributor field is never populated in this webhook handler. The GitHubComment model has a contributor field that is used in the leaderboard query (line 642), but it's not being set here. You should create or fetch a Contributor object based on the sender's GitHub ID and assign it to the comment. Look at how other webhook handlers like update_github_issues.py handle this (lines 215-224), where they use Contributor.objects.get_or_create with github_id.
if action == "closed" and is_merged and pr_user_profile:
pr_user_instance = pr_user_profile.user
assign_github_badge(pr_user_instance, "First PR Merged")
return JsonResponse({"status": "success"}, status=200)
def handle_push_event(payload):
pusher_profile = UserProfile.objects.filter(github_url=payload["sender"]["html_url"]).first()
if pusher_profile:
pusher_user = pusher_profile.user
if payload.get("commits"):
website/views/user.py:1497
- The repo field is not being populated when creating GitHubComment objects. The GitHubComment model includes a repo field (line 2335-2340 in models.py), but it's not being set in the defaults dictionary. Consider adding the repo field to ensure data consistency, similar to how it's done in the model definition.
if action == "closed" and is_merged and pr_user_profile:
pr_user_instance = pr_user_profile.user
assign_github_badge(pr_user_instance, "First PR Merged")
return JsonResponse({"status": "success"}, status=200)
def handle_push_event(payload):
pusher_profile = UserProfile.objects.filter(github_url=payload["sender"]["html_url"]).first()
if pusher_profile:
pusher_user = pusher_profile.user
if payload.get("commits"):
website/views/user.py:1443
- The contributor field is never populated in this webhook handler. The GitHubComment model has a contributor field that is used in the leaderboard query (line 642), but it's not being set here. You should create or fetch a Contributor object based on the sender's GitHub ID and assign it to the comment. Look at how other webhook handlers like update_github_issues.py handle this (lines 215-224), where they use Contributor.objects.get_or_create with github_id.
default=None,
field_name="pull_request.merged_at",
)
# --- Repo mapping (same style as handle_issue_event) ---
repo_html_url = repo_data.get("html_url")
repo_full_name = repo_data.get("full_name") # "owner/repo" (for logging only)
if not pr_global_id or not repo_html_url:
logger.warning("Pull request event missing required data (id or repo_html_url)")
return JsonResponse({"status": "error", "message": "Missing required data"}, status=400)
website/views/user.py:1443
- The repo field is not being populated when creating GitHubComment objects. The GitHubComment model includes a repo field (line 2335-2340 in models.py), but it's not being set in the defaults dictionary. Consider adding the repo field to ensure data consistency, similar to how it's done in the model definition.
default=None,
field_name="pull_request.merged_at",
)
# --- Repo mapping (same style as handle_issue_event) ---
repo_html_url = repo_data.get("html_url")
repo_full_name = repo_data.get("full_name") # "owner/repo" (for logging only)
if not pr_global_id or not repo_html_url:
logger.warning("Pull request event missing required data (id or repo_html_url)")
return JsonResponse({"status": "error", "message": "Missing required data"}, status=400)
website/views/user.py:1471
- Bot filtering logic is inconsistent with handle_issue_comment_event. In handle_issue_comment_event (line 1415-1417), there's an additional condition checking if is_bot is true AND login is not in an exceptions list. This handler only checks if login is in the bots list OR if is_bot is true, making it more restrictive. Consider aligning the bot filtering logic between both handlers for consistency.
issue_id=pr_global_id, # unique per PR (avoids clash with issues)
repo=repo,
defaults={
"type": "pull_request",
"title": pr_title,
"body": pr_body,
"state": pr_state,
"url": pr_html_url,
"is_merged": is_merged,
"created_at": created_at,
"updated_at": updated_at,
"closed_at": closed_at,
"merged_at": merged_at if is_merged else None,
"user_profile": pr_user_profile,
❌ Pre-commit checks failedThe pre-commit hooks found issues that need to be fixed. Please run the following commands locally to fix them: # Install pre-commit if you haven't already
pip install pre-commit
# Run pre-commit on all files
pre-commit run --all-files
# Or run pre-commit on staged files only
pre-commit runAfter running these commands, the pre-commit hooks will automatically fix most issues. 💡 Tip: You can set up pre-commit to run automatically on every commit by running: pre-commit installPre-commit outputFor more information, see the pre-commit documentation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (1)
website/views/user.py (1)
638-667: Populatecontributorandrepoon GitHubComment so the leaderboard isn’t always emptyThe comment leaderboard query explicitly filters for comments with a non-null
contributor:GitHubComment.objects.filter( github_issue__type="pull_request", created_at__gte=since_date, contributor__isnull=False, ... )But both
handle_issue_comment_eventandhandle_review_comment_eventonly set:
github_issueuser_profilebodycomment_typecreated_at/updated_aturlThey never populate
contributororrepo. As a result:
- Every stored comment has
contributor=None, so the leaderboard queryset always returns zero rows.- The
repofield onGitHubCommentis also left null, even though you already resolveRepoearlier in each handler.This matches the previously flagged bug and keeps the new leaderboard effectively non-functional.
You can mirror the contributor-mapping logic from
handle_pull_request_eventand setrepofrom the resolvedrepoobject. For example:Suggested fix (add contributor + repo in both handlers)
def handle_issue_comment_event(payload): @@ - # Find Issue - issue_number = issue_data.get("number") - repo_html_url = repo_data.get("html_url") - try: - repo = Repo.objects.get(repo_url=repo_html_url) - github_issue = GitHubIssue.objects.get(issue_id=issue_number, repo=repo) - except (Repo.DoesNotExist, GitHubIssue.DoesNotExist): - return JsonResponse({"status": "ignored", "reason": "Repo or Issue not found"}, status=200) + # Find Issue (prefer global GitHub ID, fall back to number for legacy rows) + issue_number = issue_data.get("number") + issue_global_id = issue_data.get("id") + repo_html_url = repo_data.get("html_url") + + try: + repo = Repo.objects.get(repo_url=repo_html_url) + except Repo.DoesNotExist: + return JsonResponse({"status": "ignored", "reason": "Repo not found"}, status=200) + + github_issue = None + if issue_global_id is not None: + github_issue = GitHubIssue.objects.filter(issue_id=issue_global_id, repo=repo).first() + if not github_issue and issue_number is not None: + github_issue = GitHubIssue.objects.filter(issue_id=issue_number, repo=repo).first() + if not github_issue: + return JsonResponse({"status": "ignored", "reason": "Issue not found"}, status=200) @@ - # Find/Create User Profile - user_profile = UserProfile.objects.filter(github_url=sender.get("html_url")).first() - - # Create Comment - GitHubComment.objects.update_or_create( - comment_id=comment_data.get("id"), - defaults={ - "github_issue": github_issue, - "user_profile": user_profile, - "body": comment_data.get("body", ""), - "comment_type": "issue_comment", - "created_at": dateutil_parser.parse(comment_data.get("created_at")), - "updated_at": dateutil_parser.parse(comment_data.get("updated_at")), - "url": comment_data.get("html_url"), - }, - ) + # Map sender to Contributor (same approach as PR handler) + user_profile = UserProfile.objects.filter(github_url=sender.get("html_url")).first() + contributor = None + gh_id = sender.get("id") + gh_login = sender.get("login") + gh_avatar = sender.get("avatar_url") + gh_github_url = sender.get("html_url") + + try: + if gh_id is not None: + contributor, _ = Contributor.objects.get_or_create( + github_id=gh_id, + defaults={ + "github_url": gh_github_url or "", + "name": gh_login or extract_github_username(gh_github_url) or "", + "avatar_url": gh_avatar or "", + "contributor_type": sender.get("type") or "User", + "contributions": 0, + }, + ) + elif gh_github_url: + contributor = Contributor.objects.filter(github_url=gh_github_url).first() + except Exception as e: + logger.error(f"Error getting/creating Contributor for issue comment: {e}") + contributor = None + + # Create / update comment + GitHubComment.objects.update_or_create( + comment_id=comment_data.get("id"), + defaults={ + "github_issue": github_issue, + "user_profile": user_profile, + "contributor": contributor, + "repo": repo, + "body": comment_data.get("body", ""), + "comment_type": "issue_comment", + "created_at": safe_parse_github_datetime( + comment_data.get("created_at"), + default=timezone.now(), + field_name="issue_comment.created_at", + ), + "updated_at": safe_parse_github_datetime( + comment_data.get("updated_at"), + default=timezone.now(), + field_name="issue_comment.updated_at", + ), + "url": comment_data.get("html_url"), + }, + ) @@ def handle_review_comment_event(payload): @@ - # Find Issue (PR is also a GitHubIssue) - pr_number = pr_data.get("number") - repo_html_url = repo_data.get("html_url") - try: - repo = Repo.objects.get(repo_url=repo_html_url) - github_issue = GitHubIssue.objects.get(issue_id=pr_number, repo=repo) - except (Repo.DoesNotExist, GitHubIssue.DoesNotExist): - return JsonResponse({"status": "ignored", "reason": "Repo or Issue not found"}, status=200) - - # Find/Create User Profile - user_profile = UserProfile.objects.filter(github_url=sender.get("html_url")).first() - - # Create Comment - GitHubComment.objects.update_or_create( - comment_id=comment_data.get("id"), - defaults={ - "github_issue": github_issue, - "user_profile": user_profile, - "body": comment_data.get("body", ""), - "comment_type": "review_comment", - "created_at": dateutil_parser.parse(comment_data.get("created_at")), - "updated_at": dateutil_parser.parse(comment_data.get("updated_at")), - "url": comment_data.get("html_url"), - }, - ) + # Find PR (GitHubIssue row) – prefer global PR ID, fall back to number + pr_number = pr_data.get("number") + pr_global_id = pr_data.get("id") + repo_html_url = repo_data.get("html_url") + + try: + repo = Repo.objects.get(repo_url=repo_html_url) + except Repo.DoesNotExist: + return JsonResponse({"status": "ignored", "reason": "Repo not found"}, status=200) + + github_issue = None + if pr_global_id is not None: + github_issue = GitHubIssue.objects.filter(issue_id=pr_global_id, repo=repo).first() + if not github_issue and pr_number is not None: + github_issue = GitHubIssue.objects.filter(issue_id=pr_number, repo=repo).first() + if not github_issue: + return JsonResponse({"status": "ignored", "reason": "PR not found"}, status=200) + + # Map sender to Contributor + user_profile = UserProfile.objects.filter(github_url=sender.get("html_url")).first() + contributor = None + gh_id = sender.get("id") + gh_login = sender.get("login") + gh_avatar = sender.get("avatar_url") + gh_github_url = sender.get("html_url") + + try: + if gh_id is not None: + contributor, _ = Contributor.objects.get_or_create( + github_id=gh_id, + defaults={ + "github_url": gh_github_url or "", + "name": gh_login or extract_github_username(gh_github_url) or "", + "avatar_url": gh_avatar or "", + "contributor_type": sender.get("type") or "User", + "contributions": 0, + }, + ) + elif gh_github_url: + contributor = Contributor.objects.filter(github_url=gh_github_url).first() + except Exception as e: + logger.error(f"Error getting/creating Contributor for review comment: {e}") + contributor = None + + GitHubComment.objects.update_or_create( + comment_id=comment_data.get("id"), + defaults={ + "github_issue": github_issue, + "user_profile": user_profile, + "contributor": contributor, + "repo": repo, + "body": comment_data.get("body", ""), + "comment_type": "review_comment", + "created_at": safe_parse_github_datetime( + comment_data.get("created_at"), + default=timezone.now(), + field_name="review_comment.created_at", + ), + "updated_at": safe_parse_github_datetime( + comment_data.get("updated_at"), + default=timezone.now(), + field_name="review_comment.updated_at", + ), + "url": comment_data.get("html_url"), + }, + )Also applies to: 1607-1663, 1666-1715
🧹 Nitpick comments (3)
website/tests/test_webhook_comments.py (1)
56-61: Extend assertions to cover fields used by the leaderboard (contributor/repo)The new leaderboard groups by
Contributorand repo, but the tests only verifygithub_issueanduser_profileonGitHubComment. This means regressions wherecontributor/repoare left null (breaking the leaderboard) would not be caught.Once the webhook handler populates these fields, consider extending the assertions, e.g.:
Suggested additional checks
- comment = GitHubComment.objects.get(comment_id=1001) - self.assertEqual(comment.github_issue, self.issue) - self.assertEqual(comment.user_profile, self.user_profile) + comment = GitHubComment.objects.get(comment_id=1001) + self.assertEqual(comment.github_issue, self.issue) + self.assertEqual(comment.user_profile, self.user_profile) + self.assertIsNotNone(comment.contributor) + self.assertEqual(comment.repo, self.repo)website/views/user.py (2)
1617-1636: Clean up bot-filter placeholder and rely on type/name checks consistentlyThe bot filters are functionally fine but contain a confusing placeholder:
if login.lower() in bots or ( is_bot and login.lower() not in ["..."] ): return JsonResponse({"status": "ignored", "reason": "Bot comment"}, status=200)
["..."]is never a real GitHub login, so this condition effectively means “ignore all senders that look like bots”, but the placeholder suggests exceptions that don’t exist.For clarity and maintainability, you can simplify to something like:
Suggested cleanup
- bots = [ - "copilot", - "coderabbitai", - "github-actions[bot]", - "dependabot", - "sonarcloud", - "vercel", - "netlify", - ] - if login.lower() in bots or ( - is_bot and login.lower() not in ["..."] - ): # Add exceptions if any bot is allowed + bots = { + "copilot", + "coderabbitai", + "github-actions[bot]", + "dependabot", + "sonarcloud", + "vercel", + "netlify", + } + # Treat everything that looks like a bot as a bot, with an explicit allowlist if needed later. + if login.lower() in bots or is_bot: return JsonResponse({"status": "ignored", "reason": "Bot comment"}, status=200)You can apply the same simplification pattern to
handle_review_comment_eventfor consistency.Also applies to: 1676-1689
658-663: Minor: redundant reassignment ofcode_review_leaderboard
context["code_review_leaderboard"]is set once at Line 636 and then set again at Line 666 without modification. The second assignment is redundant and can be dropped.This is purely cosmetic, but removing it reduces noise.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (5)
website/migrations/0261_githubcomment.pywebsite/models.pywebsite/templates/leaderboard_global.htmlwebsite/tests/test_webhook_comments.pywebsite/views/user.py
🧰 Additional context used
🧬 Code graph analysis (2)
website/tests/test_webhook_comments.py (1)
website/models.py (4)
GitHubIssue(2090-2328)Repo(1904-1981)UserProfile(896-1109)GitHubComment(2332-2366)
website/views/user.py (1)
website/models.py (4)
GitHubComment(2332-2366)Repo(1904-1981)GitHubIssue(2090-2328)UserProfile(896-1109)
⏰ 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: docker-test
- GitHub Check: Setup and Cache Dependencies
🔇 Additional comments (3)
website/migrations/0261_githubcomment.py (1)
14-64: Migration structure matches model and intended relationshipsThe
GitHubCommentmigration correctly mirrors the model (fields, FKs, ordering) and should apply cleanly given the existing app state.website/templates/leaderboard_global.html (1)
179-259: GitHub Comment Leaderboard block aligns with backend queryThe new card correctly assumes
comment_leaderboardrows shaped by.values("contributor__name", "contributor__github_url", "contributor__avatar_url")plustotal_comments, and visually matches the existing PR/Review leaderboards.Once the webhook handlers populate
GitHubComment.contributorproperly, this template should render as expected.website/models.py (1)
2332-2367: GitHubComment and BaconEarning model additions look structurally soundThe
GitHubCommentmodel’s fields and relationships (toGitHubIssue,UserProfile,Contributor,Repo) are well chosen for the leaderboard and future analytics, andBaconEarning’s timestamp/__str__improvements are straightforward.Any remaining behavior issues depend on how these models are populated (addressed in
views.user), not on the model definitions themselves.Also applies to: 2369-2375
| self.repo = Repo.objects.create(repo_url="https://github.com/owner/repo") | ||
| self.user = User.objects.create(username="testuser") | ||
| self.user_profile, created = UserProfile.objects.get_or_create(user=self.user, defaults={"github_url": "https://github.com/testuser"}) | ||
| if not created: | ||
| self.user_profile.github_url = "https://github.com/testuser" | ||
| self.user_profile.save() | ||
| self.issue = GitHubIssue.objects.create( | ||
| issue_id=1, | ||
| repo=self.repo, | ||
| title="Test PR", | ||
| type="pull_request", | ||
| state="open", | ||
| created_at="2023-01-01T00:00:00Z", | ||
| updated_at="2023-01-01T00:00:00Z" | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix Repo and GitHubIssue creation to satisfy model constraints and match production semantics
Repo and GitHubIssue are created with required fields missing and with non-idiomatic datetime types:
Repo.nameis non-nullable, butRepo.objects.create(repo_url=...)omits it.GitHubIssue.urlis non-nullable, butGitHubIssue.objects.create(...)omits it.created_at/updated_atare passed as strings, while the model expectsdatetimeinstances.
Given the current Repo and GitHubIssue definitions in website/models.py, this will raise integrity errors at save time and diverges from how PR issues are actually persisted in production.
Consider updating setUp along these lines:
Suggested fix
- self.repo = Repo.objects.create(repo_url="https://github.com/owner/repo")
+ self.repo = Repo.objects.create(
+ name="owner/repo",
+ repo_url="https://github.com/owner/repo",
+ )
@@
- self.issue = GitHubIssue.objects.create(
- issue_id=1,
- repo=self.repo,
- title="Test PR",
- type="pull_request",
- state="open",
- created_at="2023-01-01T00:00:00Z",
- updated_at="2023-01-01T00:00:00Z"
- )
+ from django.utils import timezone
+ now = timezone.now()
+
+ self.issue = GitHubIssue.objects.create(
+ issue_id=1, # test assumes issue_id == PR number; see comment-handler lookup
+ repo=self.repo,
+ title="Test PR",
+ url="https://github.com/owner/repo/pull/1",
+ type="pull_request",
+ state="open",
+ created_at=now,
+ updated_at=now,
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| self.repo = Repo.objects.create(repo_url="https://github.com/owner/repo") | |
| self.user = User.objects.create(username="testuser") | |
| self.user_profile, created = UserProfile.objects.get_or_create(user=self.user, defaults={"github_url": "https://github.com/testuser"}) | |
| if not created: | |
| self.user_profile.github_url = "https://github.com/testuser" | |
| self.user_profile.save() | |
| self.issue = GitHubIssue.objects.create( | |
| issue_id=1, | |
| repo=self.repo, | |
| title="Test PR", | |
| type="pull_request", | |
| state="open", | |
| created_at="2023-01-01T00:00:00Z", | |
| updated_at="2023-01-01T00:00:00Z" | |
| ) | |
| self.repo = Repo.objects.create( | |
| name="owner/repo", | |
| repo_url="https://github.com/owner/repo", | |
| ) | |
| self.user = User.objects.create(username="testuser") | |
| self.user_profile, created = UserProfile.objects.get_or_create(user=self.user, defaults={"github_url": "https://github.com/testuser"}) | |
| if not created: | |
| self.user_profile.github_url = "https://github.com/testuser" | |
| self.user_profile.save() | |
| from django.utils import timezone | |
| now = timezone.now() | |
| self.issue = GitHubIssue.objects.create( | |
| issue_id=1, # test assumes issue_id == PR number; see comment-handler lookup | |
| repo=self.repo, | |
| title="Test PR", | |
| url="https://github.com/owner/repo/pull/1", | |
| type="pull_request", | |
| state="open", | |
| created_at=now, | |
| updated_at=now, | |
| ) |
🤖 Prompt for AI Agents
In website/tests/test_webhook_comments.py around lines 10-24, the test creates
Repo and GitHubIssue without required fields and passes datetimes as strings;
update setUp to provide Repo.name when creating the repo, provide
GitHubIssue.url (e.g. a full GitHub issue/PR URL) when creating the issue, and
supply proper datetime instances (UTC-aware datetimes or use
django.utils.timezone.now()/timezone.make_aware) for created_at and updated_at
so the objects satisfy model constraints and mirror production semantics.
| issue_number = issue_data.get("number") | ||
| repo_html_url = repo_data.get("html_url") | ||
| try: | ||
| repo = Repo.objects.get(repo_url=repo_html_url) | ||
| github_issue = GitHubIssue.objects.get(issue_id=issue_number, repo=repo) | ||
| except (Repo.DoesNotExist, GitHubIssue.DoesNotExist): | ||
| return JsonResponse({"status": "ignored", "reason": "Repo or Issue not found"}, status=200) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's check if the file exists and get its structure
wc -l website/views/user.pyRepository: OWASP-BLT/BLT
Length of output: 83
🏁 Script executed:
# Examine the specific line ranges mentioned
echo "=== Lines 1639-1645 ==="
sed -n '1639,1645p' website/views/user.py
echo -e "\n=== Lines 1691-1698 ==="
sed -n '1691,1698p' website/views/user.py
echo -e "\n=== Lines 1368-1375 ==="
sed -n '1368,1375p' website/views/user.pyRepository: OWASP-BLT/BLT
Length of output: 1406
🏁 Script executed:
# Find the handle_pull_request_event function
grep -n "def handle_pull_request_event" website/views/user.pyRepository: OWASP-BLT/BLT
Length of output: 101
🏁 Script executed:
# Find comment handler functions
grep -n "def handle.*comment" website/views/user.pyRepository: OWASP-BLT/BLT
Length of output: 149
🏁 Script executed:
# Search for where GitHubIssue is created with issue_id
rg "GitHubIssue.*issue_id" website/views/user.py -A 2 -B 2Repository: OWASP-BLT/BLT
Length of output: 1504
🏁 Script executed:
# Find where GitHubIssue is created/updated in handle_pull_request_event
# Function starts at line 1348, let's look at its body
sed -n '1348,1550p' website/views/user.py | head -150Repository: OWASP-BLT/BLT
Length of output: 6299
🏁 Script executed:
# Let's search more specifically for where issue_id is set in handle_pull_request_event
sed -n '1348,1500p' website/views/user.py | grep -A 5 -B 5 "issue_id"Repository: OWASP-BLT/BLT
Length of output: 1118
🏁 Script executed:
# Look at the full context of the PR handling function to see what's created
sed -n '1348,1550p' website/views/user.pyRepository: OWASP-BLT/BLT
Length of output: 8295
🏁 Script executed:
# Look at the full context of handle_issue_comment_event around line 1639
sed -n '1607,1665p' website/views/user.pyRepository: OWASP-BLT/BLT
Length of output: 2295
🏁 Script executed:
# Look at the full context of handle_review_comment_event around line 1691
sed -n '1666,1730p' website/views/user.pyRepository: OWASP-BLT/BLT
Length of output: 2458
🏁 Script executed:
# Check if there's a fallback pattern anywhere in comment handlers
sed -n '1607,1750p' website/views/user.py | grep -A 10 "issue_id"Repository: OWASP-BLT/BLT
Length of output: 1032
Comment handlers will silently fail to find PRs created by the new webhook handler due to ID mismatch
The PR webhook handler creates GitHubIssue records with the global PR ID (pr_global_id), but both comment handlers search by the visible PR number (issue_number / pr_number). In real GitHub payloads these differ, so comments on PRs created via the new handler will not attach—the lookup fails and returns "Repo or Issue not found" without error.
The same mismatch affects all three locations: lines 1639-1645, 1691-1698, and 1368-1375. A fix should prioritize the global ID lookup with a fallback to visible number for legacy compatibility.
🤖 Prompt for AI Agents
In website/views/user.py around lines 1639-1645 (and similarly at 1691-1698 and
1368-1375), the handlers currently only look up GitHubIssue by the visible
issue/PR number, which fails for PRs created with the new webhook that stores
the global PR ID; change the lookup to first attempt finding GitHubIssue by the
global PR id field stored by the new handler (e.g. pr_global_id or node_id from
the payload) and only if that fails fall back to looking up by the visible issue
number, keeping the existing Repo lookup and preserving behavior for legacy
records; apply this change to all three locations mentioned.
| comment_id=comment_data.get("id"), | ||
| defaults={ | ||
| "github_issue": github_issue, | ||
| "user_profile": user_profile, | ||
| "body": comment_data.get("body", ""), | ||
| "comment_type": "issue_comment", | ||
| "created_at": dateutil_parser.parse(comment_data.get("created_at")), | ||
| "updated_at": dateutil_parser.parse(comment_data.get("updated_at")), | ||
| "url": comment_data.get("html_url"), | ||
| }, | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Webhook handlers for comments fail to populate the contributor field, which will break the comment leaderboard feature as it filters for non-null contributors.
Severity: CRITICAL | Confidence: High
🔍 Detailed Analysis
The webhook handlers handle_issue_comment_event and handle_review_comment_event do not populate the contributor field when creating or updating GithubComment objects. The leaderboard query filters records using contributor__isnull=False. Consequently, any comments created via these webhooks will be excluded from the query results, causing the leaderboard feature to fail and display no data.
💡 Suggested Fix
In the handle_issue_comment_event and handle_review_comment_event functions, retrieve the Contributor object associated with the comment's author. Then, include this contributor object in the defaults dictionary when calling update_or_create() for the GithubComment model.
🤖 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: website/views/user.py#L1648-L1658
Potential issue: The webhook handlers `handle_issue_comment_event` and
`handle_review_comment_event` do not populate the `contributor` field when creating or
updating `GithubComment` objects. The leaderboard query filters records using
`contributor__isnull=False`. Consequently, any comments created via these webhooks will
be excluded from the query results, causing the leaderboard feature to fail and display
no data.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 7916320
❌ Tests failedThe Django tests found issues that need to be fixed. Please review the test output below and fix the failing tests. How to run tests locally# Install dependencies
poetry install --with dev
# Run all tests
poetry run python manage.py test
# Run tests with verbose output
poetry run python manage.py test -v 3
# Run a specific test
poetry run python manage.py test app.tests.TestClass.test_methodTest outputFor more information, see the Django testing documentation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
website/views/user.py (1)
559-665: Comment leaderboard will remain empty because webhook handlers don't populate thecontributorfield onGitHubCommentThe
handle_issue_comment_eventandhandle_review_comment_eventhandlers create/update comments withuser_profile, but never setcontributor. Unlike the PR webhook handler which explicitly gets or creates aContributorobject, the comment handlers omit this entirely. The leaderboard query filters oncontributor__isnull=False, so it will return no results untilcontributoris populated when comments are created.
♻️ Duplicate comments (4)
website/tests/test_webhook_comments.py (3)
60-110: Add coverage for pull_request_review_comment and strengthen bot/filter testsRight now this module only tests
issue_commentevents. The newhandle_review_comment_eventhandler and thepull_request_review_commentrouting path are untested, and bugs there (e.g., missing contributor mapping, ID mismatch) won’t be caught.Consider:
- Adding test cases for
pull_request_review_commentevents:
- happy-path PR review comment creating
GitHubCommentwith correctgithub_issue,user_profile, andcontributor.- bot review comments being ignored.
- Reusing the same HMAC helper and setup as for the issue_comment tests.
This will close the gap around the second comment handler and keep the leaderboard behavior well-covered.
9-29: Fix Repo/GitHubIssue setup to satisfy model constraints and mirror production
RepoandGitHubIssueare created with required fields missing and non-idiomatic datetime types:
Repo.nameis required, butRepo.objects.create(repo_url=...)omits it.GitHubIssue.urlis required but not provided.created_at/updated_atare passed as strings instead of timezone-aware datetimes.This is very likely to raise integrity/validation errors and diverges from how PR issues are stored in production.
Consider updating
setUpalong these lines:Suggested fix
- self.repo = Repo.objects.create(repo_url="https://github.com/owner/repo") + self.repo = Repo.objects.create( + name="owner/repo", + repo_url="https://github.com/owner/repo", + ) @@ - self.issue = GitHubIssue.objects.create( - issue_id=1, - repo=self.repo, - title="Test PR", - type="pull_request", - state="open", - created_at="2023-01-01T00:00:00Z", - updated_at="2023-01-01T00:00:00Z", - ) + from django.utils import timezone + now = timezone.now() + + self.issue = GitHubIssue.objects.create( + issue_id=1, # for tests you can keep this aligned with the PR number + repo=self.repo, + title="Test PR", + url="https://github.com/owner/repo/pull/1", + type="pull_request", + state="open", + created_at=now, + updated_at=now, + )
31-59: Webhook tests need HMAC signature + should assert contributor linkageThe
github_webhookview enforces a configuredGITHUB_WEBHOOK_SECRETand validatesX-Hub-Signature-256. These tests POST without a secret override or signature header, so they will return 503/403 whenever the secret is set in settings.Additionally, you only assert
github_issueanduser_profileon the created comment; the leaderboard depends onGitHubComment.contributor, so tests should verify that is populated.Suggested changes:
- Decorate the test class with
@override_settings(GITHUB_WEBHOOK_SECRET="test-secret").- Add a helper (mirroring
test_github_webhook.py) to compute the HMAC and setHTTP_X_HUB_SIGNATURE_256.- Replace direct
self.client.post(...)calls with that helper.- In this test, also assert that
comment.contributoris notNoneand has expected values once handlers set it.website/views/user.py (1)
1605-1712: Fix ID lookup mismatch and populatecontributor/repoon GitHubCommentTwo functional problems here will make the comment leaderboard effectively non-functional for newly tracked PRs:
ID mismatch between PR creation and comment lookup (real bug)
handle_pull_request_eventstores PRs withGitHubIssue.issue_id = pr_global_id(the global PR ID frompull_request["id"]).Both comment handlers look up by the visible PR/issue number:
issue_number = issue_data.get("number") github_issue = GitHubIssue.objects.get(issue_id=issue_number, repo=repo) # ... pr_number = pr_data.get("number") github_issue = GitHubIssue.objects.get(issue_id=pr_number, repo=repo)For PRs created via
handle_pull_request_event, this lookup will not find them, and you’ll return"Repo or Issue not found"while silently dropping comments. To keep compatibility:
- First attempt lookup by the global ID present in the payload (
issue_data.get("id")/pr_data.get("id"), or the PR global id if you include it), matching what you use inhandle_pull_request_event.- Fall back to the legacy
number-based lookup only if that fails, so any older data keyed by number continues to work.
GitHubComment.contributor(andrepo) are never set (leaderboard blocker)
The leaderboard explicitly filtersGitHubCommentbycontributor__isnull=False, but here you only setgithub_issueanduser_profile. That means every stored comment currently hascontributor = NULLand is filtered out.You should mirror the contributor-mapping logic already used in
handle_pull_request_event:sender = payload.get("sender") or {} gh_id = sender.get("id") gh_login = sender.get("login") gh_avatar = sender.get("avatar_url") gh_html_url = sender.get("html_url") contributor = None try: if gh_id is not None: contributor, _ = Contributor.objects.get_or_create( github_id=gh_id, defaults={ "github_url": gh_html_url or "", "name": gh_login or extract_github_username(gh_html_url) or "", "avatar_url": gh_avatar or "", "contributor_type": sender.get("type") or "User", "contributions": 0, }, ) elif gh_html_url: contributor = Contributor.objects.filter(github_url=gh_html_url).first() except Exception as e: logger.error("Error getting/creating Contributor for comment: %s", e) contributor = NoneThen include both
contributorandrepoin thedefaultsforGitHubComment.objects.update_or_create(...)in both handlers:defaults={ "github_issue": github_issue, "user_profile": user_profile, "contributor": contributor, "repo": repo, ... }Optional hardening: use
safe_parse_github_datetimeforcreated_at/updated_aton comments, just like you do for PR timestamps, so malformed or missing values don’t blow up the handler.A concrete minimal patch for the ID lookup part of
handle_issue_comment_eventcould look like:Suggested ID lookup fix (apply similarly to
handle_review_comment_event)- issue_number = issue_data.get("number") - repo_html_url = repo_data.get("html_url") - try: - repo = Repo.objects.get(repo_url=repo_html_url) - github_issue = GitHubIssue.objects.get(issue_id=issue_number, repo=repo) - except (Repo.DoesNotExist, GitHubIssue.DoesNotExist): - return JsonResponse({"status": "ignored", "reason": "Repo or Issue not found"}, status=200) + issue_number = issue_data.get("number") + issue_global_id = issue_data.get("id") + repo_html_url = repo_data.get("html_url") + try: + repo = Repo.objects.get(repo_url=repo_html_url) + github_issue = None + # Prefer global ID (matches handle_pull_request_event), fall back to number for legacy rows + if issue_global_id is not None: + github_issue = GitHubIssue.objects.filter(issue_id=issue_global_id, repo=repo).first() + if not github_issue and issue_number is not None: + github_issue = GitHubIssue.objects.filter(issue_id=issue_number, repo=repo).first() + if not github_issue: + return JsonResponse({"status": "ignored", "reason": "Repo or Issue not found"}, status=200) + except Repo.DoesNotExist: + return JsonResponse({"status": "ignored", "reason": "Repo not found"}, status=200) + except Exception as e: + logger.error("Error resolving repo/issue for issue_comment: %s", e) + return JsonResponse({"status": "error"}, status=500)Applying the same pattern plus contributor/repo defaults in both handlers will make the webhook path and leaderboard consistent with the new PR storage semantics.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (5)
website/migrations/0261_githubcomment.pywebsite/models.pywebsite/templates/leaderboard_global.htmlwebsite/tests/test_webhook_comments.pywebsite/views/user.py
🚧 Files skipped from review as they are similar to previous changes (1)
- website/templates/leaderboard_global.html
🧰 Additional context used
🧬 Code graph analysis (2)
website/tests/test_webhook_comments.py (1)
website/models.py (14)
GitHubComment(2331-2365)GitHubIssue(2090-2328)Repo(1904-1981)UserProfile(896-1109)save(74-77)save(277-289)save(1400-1427)save(1535-1538)save(1728-1731)save(1846-1860)save(1951-1973)save(2809-2812)save(3341-3351)save(3649-3658)
website/views/user.py (1)
website/models.py (4)
GitHubComment(2331-2365)Repo(1904-1981)GitHubIssue(2090-2328)UserProfile(896-1109)
⏰ 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). (7)
- GitHub Check: auto-assign
- GitHub Check: remove_last_active_label
- GitHub Check: add_changes_requested_label
- GitHub Check: check_peer_review
- GitHub Check: remove_last_active_label
- GitHub Check: Run Tests
- GitHub Check: docker-test
🔇 Additional comments (4)
website/models.py (1)
2331-2365: GitHubComment model shape looks correct and migration-alignedFields, FKs, and ordering align with how you query in the leaderboard and webhook handlers; no issues from a model/schema perspective.
website/migrations/0261_githubcomment.py (1)
1-63: Migration matches model and should apply cleanlyThe GitHubComment table definition matches the model (including FKs and ordering), so schema-wise this is consistent.
website/views/user.py (2)
20-63: Imports and GitHubComment wiring are consistentBringing in
MaxandGitHubCommenthere matches how you aggregate comments and route webhook events later in the file; no issues.
1322-1332: Routing new GitHub events into dedicated handlers looks correctAdding
"issue_comment"and"pull_request_review_comment"to the event dispatch table cleanly integrates the new handlers without affecting existing events.
|
💬 Reminder: Unresolved Conversations Hi @mdkaifansari04! This pull request has 4 unresolved conversations that need to be addressed. Please review and resolve the pending discussions so we can move forward with merging this PR. Thank you! 🙏 |
|
💬 Reminder: Unresolved Conversations Hi @mdkaifansari04! This pull request has 4 unresolved conversations that need to be addressed. Please review and resolve the pending discussions so we can move forward with merging this PR. Thank you! 🙏 |
Fixes: #4662
Description
This pr adds a github pr comment leaderboard to the global leaderboard page.
it shows contributors who actively comment on prs across blt repos.
note: the screenshot uses test data, so activity and dates may look random.
how it works
what it shows
maintainer notes
why this matters
code reviews matter. this highlights people who show up, help others, and improve prs.
it rewards real engagement, not just commits.
Summary by CodeRabbit
New Features
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.