Implement GraphQL queries for contributor recognition certificate pages#4962
Open
anurag2787 wants to merge 16 commits into
Open
Implement GraphQL queries for contributor recognition certificate pages#4962anurag2787 wants to merge 16 commits into
anurag2787 wants to merge 16 commits into
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
Summary by CodeRabbitRelease Notes
WalkthroughAdds a Contributor Recognition Program (CRP) subsystem: renames four CRP model DB tables to ChangesCRP Certificate & Score System
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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 `@backend/apps/github/models/generic_issue_model.py`:
- Around line 26-29: The StateReason enum class is incomplete and only includes
the COMPLETED choice. Add the additional state_reason values supported by
GitHub's API: NOT_PLANNED with value "not_planned", REOPENED with value
"reopened", and DUPLICATE with value "duplicate" to the StateReason enum in the
same format as the existing COMPLETED entry. This will ensure consistency with
the GitHub API and enable better IDE autocompletion when filtering by
state_reason fields in the future.
In `@backend/apps/owasp/admin/contribution_score.py`:
- Around line 46-69: The recalculate method processes potentially many users
without providing intermediate progress feedback, which could cause admins to
think the operation is stuck. Add periodic progress messages inside the loop in
the recalculate method by calling self.message_user with messages.INFO level at
regular intervals (e.g., every 100 processed users). You will need to import
messages from django.contrib at the top of the file to enable this.
In `@backend/apps/owasp/api/internal/queries/certificate.py`:
- Line 18: The parameter name `id` in the certificate method shadows Python's
builtin id() function. Rename the parameter from `id` to `certificate_id` (or
`pk` if preferred) and update all references to this parameter throughout the
method body to use the new name. Also check if this method is called elsewhere
in the codebase and update the argument names at those call sites to match the
renamed parameter.
In `@backend/apps/owasp/score_calculator.py`:
- Around line 358-392: The recalculate_user method does not handle exceptions
from CertificateService.issue_certificate(), allowing them to propagate and
abort the operation, while recalculate_all() catches and tracks these failures.
Either wrap the CertificateService.issue_certificate(user, total_score, tier)
call in a try/except block to match the error handling pattern used in
recalculate_all() by capturing the exception and logging it without aborting the
score update, or add clear documentation explaining the intentional difference
in error handling behavior between single-user and batch recalculation
operations.
- Around line 227-233: In the User query filter chain in the
`score_calculator.py` file, replace the `prefetch_related("contribution_score")`
call with `select_related("contribution_score")`. Since `contribution_score` is
a OneToOneField relation, using select_related will perform a JOIN in a single
query, which is more efficient than prefetch_related's separate query approach.
- Around line 163-182: The method `count_opened_issues` contains a filter for
`state_reason=Issue.StateReason.COMPLETED` which means it actually counts
completed issues, not opened issues, making the method name misleading. Rename
the method from `count_opened_issues` to `count_completed_issues` to accurately
reflect its behavior. Then locate and update all callers of this method
(including the call in `get_contribution_breakdown`) to use the new method name.
In `@backend/apps/owasp/services/certificate_providers.py`:
- Around line 44-48: The issue() method in the certificate_providers.py file
creates a Certificate instance via Certificate.objects.create() but does not
return it. Capture the return value from the Certificate.objects.create() call
and return it from the issue() method so callers can access certificate metadata
like the generated ID. Additionally, update the abstract base class signature to
reflect that the issue() method now returns a Certificate instance instead of
None.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: bee30a1f-bc1a-463b-890a-3dc75a8af70b
📒 Files selected for processing (26)
backend/apps/github/api/internal/nodes/user.pybackend/apps/github/models/generic_issue_model.pybackend/apps/owasp/Makefilebackend/apps/owasp/admin/certificate.pybackend/apps/owasp/admin/contribution_score.pybackend/apps/owasp/admin/leaderboard_snapshot.pybackend/apps/owasp/admin/scoring_weight.pybackend/apps/owasp/api/internal/nodes/certificate.pybackend/apps/owasp/api/internal/nodes/contribution_score.pybackend/apps/owasp/api/internal/queries/__init__.pybackend/apps/owasp/api/internal/queries/certificate.pybackend/apps/owasp/exceptions.pybackend/apps/owasp/management/commands/owasp_crp_recognition_recalculate_scores.pybackend/apps/owasp/migrations/0074_alter_certificate_table_and_more.pybackend/apps/owasp/models/__init__.pybackend/apps/owasp/models/crp/__init__.pybackend/apps/owasp/models/crp/certificate.pybackend/apps/owasp/models/crp/contribution_score.pybackend/apps/owasp/models/crp/leaderboard_snapshot.pybackend/apps/owasp/models/crp/recognition_enums.pybackend/apps/owasp/models/crp/scoring_weight.pybackend/apps/owasp/score_calculator.pybackend/apps/owasp/services/__init__.pybackend/apps/owasp/services/certificate_providers.pybackend/apps/owasp/services/certificate_service.pycspell/custom-dict.txt
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
5 issues found across 26 files
Confidence score: 3/5
- In
backend/apps/owasp/score_calculator.py, theissue_openedpath (includingcount_opened_issues) filters onstate_reason=COMPLETED, which can undercount contributors with open issues and even skip them during recalculation; merging as-is risks incorrect contribution scores and recognition outcomes — fix the filter/logic to count genuinely opened issues before merging. - In
backend/apps/owasp/api/internal/nodes/contribution_score.py,ContributionScoreNodeomits thetierfield, so clients (including certificate-related consumers) may receive incomplete contribution data and render outdated or missing tier info — addtierto the node/model mapping before merge. - In
backend/apps/owasp/score_calculator.py, usingprefetch_relatedfor reverse OneToOne loading adds unnecessary query overhead during full recalculation, andbackend/apps/owasp/management/commands/owasp_crp_recognition_recalculate_scores.pylogs a broader scope than actually processed; this is lower risk but can hurt operability and debugging — switch toselect_relatedand correct the command status message as a cleanup.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Collaborator
Author
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## feature/contributor-recognition-program #4962 +/- ##
===========================================================================
- Coverage 98.74% 97.88% -0.86%
===========================================================================
Files 547 554 +7
Lines 17185 17436 +251
Branches 2417 2435 +18
===========================================================================
+ Hits 16969 17067 +98
- Misses 127 280 +153
Partials 89 89
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
arkid15r
requested changes
Jun 26, 2026
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
2 participants
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.
Proposed change
This PR implements GraphQL queries and nodes required for the Contributor Recognition certificate pages.
Resolves #4949
Checklist
make check-testlocally: all warnings addressed, tests passed