fix: repair dead discussion mentor-counting branch#776
Open
kaizeenn wants to merge 2 commits into
Open
Conversation
Three layered defects made `enable_mentor_count` a no-op (or crash) for discussions (reported in github-community-projects#774): 1. GraphQL fetched no comment author data discussions.py queried `comments(first: 1) { nodes { createdAt } }`, so each comment node had no author field. Expanded to `comments(first: 100)` with `author { login __typename }`, and added `author { login __typename }` to the discussion node itself so the opener's login is available for the self-comment filter. 2. Attribute access on dict nodes would AttributeError most_active_mentors.py used `comment.user.login`, `comment.submitted_at` etc. on plain GraphQL dicts. Replaced with `comment.get('author') or {}` dict access throughout. 3. Self-reference in ignore_comment The old code passed `comment.user` as both `issue_user` AND `comment_user`, so the 'same user' guard always fired and every comment was skipped. Replaced by threading `discussion['author']['login']` as the discussion-author login and comparing directly. Also moved the discussion block outside the `if issue:` guard so it runs when `issue=None` (the call-site in issue_metrics.py passes `issue=None, discussion=<dict>`). Added TestCountCommentsDiscussions with 7 cases: basic count, author self-comment ignored, bot ignored, ignore_users respected, multiple commenters, empty comments, null author (deleted account). Closes github-community-projects#774 Signed-off-by: kaizeenn <khairil0153@gmail.com>
Collaborator
|
@kaizeenn looks like we have some python linting failing. |
Contributor
There was a problem hiding this comment.
Pull request overview
Repairs the previously non-functional “mentor counting” path for GitHub Discussions by expanding the GraphQL payload and updating count_comments_per_user to correctly process Discussion comment dicts (including self-comment and bot filtering), with added unit coverage for the discussion branch.
Changes:
- Expand
get_discussionsGraphQL query to include discussion/comment authors and fetch up to 100 comments per discussion. - Rewrite the Discussions branch in
count_comments_per_userto use dict-safe access and correct self-comment filtering. - Add a dedicated
TestCountCommentsDiscussionssuite covering core counting and ignore behaviors.
Show a summary per file
| File | Description |
|---|---|
discussions.py |
Expands GraphQL fields for discussion + comment authors and increases fetched comment nodes to support mentor counting. |
most_active_mentors.py |
Fixes the discussion mentor-counting branch to work with GraphQL dict payloads and correct ignore logic. |
test_most_active_mentors.py |
Adds 7 tests covering discussion mentor counting scenarios (self, bots, ignore list, null author, etc.). |
Copilot's findings
- Files reviewed: 3/3 changed files
- Comments generated: 2
Comment on lines
+118
to
+123
| if discussion and len(discussion["comments"]["nodes"]) > 0: | ||
| discussion_author_login = ( | ||
| discussion.get("author") or {} | ||
| ).get("login", "") | ||
| for comment in discussion["comments"]["nodes"]: | ||
| comment_author = comment.get("author") or {} |
Comment on lines
+141
to
+144
| if comment_login in mentor_count: | ||
| mentor_count[comment_login] += 1 | ||
| else: | ||
| mentor_count[comment_login] = 1 |
Author
Collaborator
|
Hey @kaizeenn! It looks like the copilot review bot raised some good points. Could you take a look at the fixes for those? |
Collaborator
|
🤖 Now that the PyGithub migration (#789) has landed on main, this PR will need a rebase. The good news is that the actual logic in the PR is still correct and does not need to change. Here is what to expect when rebasing:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #774.
Three layered defects made
enable_mentor_counta complete no-op (or a crash) for GitHub Discussions:1. GraphQL fetched no comment author data
discussions.pyqueriedcomments(first: 1) { nodes { createdAt } }— noauthorfield, and only 1 comment. Expanded tocomments(first: 100)withauthor { login __typename }on each comment node. Addedauthor { login __typename }to the discussion node itself so the opener's login is available for the self-comment filter.2. Attribute access on dict nodes would AttributeError
most_active_mentors.pyusedcomment.user.login,comment.submitted_at,comment.ready_for_review_aton plain GraphQL dicts. Replaced withcomment.get('author') or {}dict access throughout.3. Self-reference in
ignore_commentThe old code passed
comment.useras bothissue_userandcomment_user, so the guardcomment_user.login == issue_user.loginalways fired and every comment was skipped. Fixed by threadingdiscussion['author']['login']as the discussion-author login and comparing directly, without going throughignore_comment(which expects github3 objects, not dicts).Structural fix
Moved the discussion block outside the
if issue:guard so it actually runs when the call-site passesissue=None, discussion=<dict>(asissue_metrics.pydoes).Development notes
Changed files:
discussions.py,most_active_mentors.py,test_most_active_mentors.py.Added
TestCountCommentsDiscussionswith 7 cases:ignore_usersrespectedpytest test_most_active_mentors.py— 11 passed (4 baseline + 7 new). Commit signed off (DCO).