Skip to content

Conversation

@samyak003
Copy link
Contributor

Fixes: #1353

@samyak003 samyak003 requested a review from arkid15r as a code owner April 16, 2025 19:11
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Apr 16, 2025

Summary by CodeRabbit

  • New Features

    • Enhanced top contributors data with additional filtering options by chapter, committee, project, or repository.
    • Contributor information now includes project-specific details where applicable.
  • Refactor

    • Top contributors data is now queried and managed independently from main entity objects (chapters, committees, projects, repositories) in both the backend and frontend.
    • Improved consistency and clarity in how top contributors are retrieved and displayed across different pages.
  • Bug Fixes

    • Adjusted frontend and test data structures to align with updated backend and query responses, ensuring accurate display of contributor information.
  • Tests

    • Updated unit and end-to-end tests to reflect the new data structures for top contributors.

Summary by CodeRabbit

  • New Features
    • Added the ability to filter top contributors by project, chapter, committee, and repository in the contributors query. This allows users to view top contributors for specific entities in addition to existing organization-based filtering.
  • Refactoring
    • Centralized the top contributors retrieval logic into a new class method RepositoryContributor.get_top_contributors. This method is now used by the GraphQL query resolver, the RepositoryIndexMixin property, and the OWASP model mixins, eliminating duplicated inline query logic across the codebase.
    • Updated frontend components to manage topContributors state separately from main entity objects and adjusted GraphQL queries accordingly.
    • Updated tests and mock data to reflect the new data shape where topContributors is queried independently rather than nested inside entity objects.

Walkthrough

The changes introduce optional filter arguments (chapter, committee, project, repository) to the top_contributors GraphQL query and update its resolver to delegate fetching contributors to a new class method RepositoryContributor.get_top_contributors. The idx_top_contributors properties in repository, chapter, committee, and project mixins were refactored to use this centralized method, removing redundant inline query logic. Frontend components and GraphQL queries were updated to fetch and manage top contributors data as a separate query field, decoupled from main entity objects. Corresponding test mocks were adjusted to match the new data structure.

Changes

File(s) Change Summary
backend/apps/github/graphql/queries/repository_contributor.py Added optional arguments (chapter, committee, project, repository) to top_contributors query; updated resolver to delegate fetching to RepositoryContributor.get_top_contributors.
backend/apps/github/models/repository_contributor.py Added class method get_top_contributors supporting filters by chapter, committee, organization, project, and repository.
backend/apps/github/models/mixins/repository.py Refactored idx_top_contributors property to use RepositoryContributor.get_top_contributors instead of inline query logic.
backend/apps/owasp/models/common.py Removed old get_top_contributors method; replaced usages with calls to RepositoryContributor.get_top_contributors.
backend/apps/owasp/models/mixins/chapter.py Refactored idx_top_contributors property to call RepositoryContributor.get_top_contributors with chapter key.
backend/apps/owasp/models/mixins/committee.py Refactored idx_top_contributors property to call RepositoryContributor.get_top_contributors with committee key.
backend/apps/owasp/models/mixins/project.py Refactored idx_top_contributors property to call RepositoryContributor.get_top_contributors with project key.
frontend/src/app/chapters/[chapterKey]/page.tsx Added local state for topContributors and updated component to use it instead of nested chapter property.
frontend/src/app/committees/[committeeKey]/page.tsx Added local state for topContributors and updated component to use it instead of nested committee property.
frontend/src/app/organizations/[organizationKey]/repositories/[repositoryKey]/page.tsx Added local state for topContributors and updated component to use it instead of nested repository property.
frontend/src/app/projects/[projectKey]/page.tsx Added local state for topContributors and updated component to use it instead of nested project property.
frontend/src/server/queries/chapterQueries.ts Moved topContributors field out of nested chapter object to a separate root-level query with chapter argument.
frontend/src/server/queries/committeeQueries.ts Moved topContributors field out of nested committee object to a separate root-level query with committee argument.
frontend/src/server/queries/projectQueries.ts Moved topContributors field out of nested project object to a separate root-level query with project argument.
frontend/src/server/queries/repositoryQueries.ts Moved topContributors field out of nested repository object to a separate root-level query with repository and organization arguments.
frontend/tests/e2e/pages/ChapterDetails.spec.ts Updated mocked GraphQL response shape to remove nested chapter key, matching new query structure.
frontend/tests/unit/data/mockChapterDetailsData.ts Nested chapter-related properties inside a new chapter object; duplicated topContributors at root level.
frontend/tests/unit/data/mockCommitteeDetailsData.ts Moved topContributors out of nested committee object to a top-level property; removed __typename fields.
frontend/tests/unit/data/mockProjectDetailsData.ts Moved topContributors from nested project object to top-level property.
frontend/tests/unit/data/mockRepositoryData.ts Moved topContributors from nested repository object to top-level property.
frontend/tests/unit/pages/ChapterDetails.test.tsx Updated mocked useQuery return values to match new data shape without nested chapter key.
frontend/tests/unit/pages/CommitteeDetails.test.tsx Updated mocked useQuery return values to match new data shape with committee and topContributors as siblings.

Assessment against linked issues

Objective (Issue #) Addressed Explanation
Unify top contributors logic in a single GraphQL endpoint and avoid code duplication (#1353)

Suggested labels

backend-tests

Tip

⚡️ Faster reviews with caching
  • CodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure Review - Disable Cache at either the organization or repository level. If you prefer to disable all data retention across your organization, simply turn off the Data Retention setting under your Organization Settings.

Enjoy the performance boost—your workflow just got faster.


📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e9b917c and 8f61255.

📒 Files selected for processing (1)
  • frontend/__tests__/unit/data/mockChapterDetailsData.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • frontend/tests/unit/data/mockChapterDetailsData.ts
⏰ Context from checks skipped due to timeout of 90000ms (6)
  • GitHub Check: Run frontend unit tests
  • GitHub Check: Run backend tests
  • GitHub Check: Run Code Scan
  • GitHub Check: Run CI Denendencies Scan
  • GitHub Check: CodeQL (python)
  • GitHub Check: CodeQL (javascript-typescript)
✨ 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.
    • Explain this complex logic.
    • 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 explain this code block.
    • @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 explain its main purpose.
    • @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.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

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 generate sequence diagram to generate a sequence diagram of the changes in 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.

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.

Copy link
Collaborator

@arkid15r arkid15r left a comment

Choose a reason for hiding this comment

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

We should take a look at these cases and refactor/consolide the code making sure that DRY principle is followed throughout the entire code base.

// Feel free to PM me if you have more questions.

@arkid15r arkid15r marked this pull request as draft April 18, 2025 17:46
@samyak003 samyak003 requested review from arkid15r April 19, 2025 22:15
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (1)
backend/apps/github/models/repository_contributor.py (1)

140-143: Simplify redundant condition in project filter.

The project filter has a redundant condition that can be simplified.

-            queryset = queryset.filter(
-                repository__project__key__iexact=f"www-project-{project}" if project else None
-            )
+            queryset = queryset.filter(
+                repository__project__key__iexact=f"www-project-{project}"
+            )

The outer if project: check already ensures this code only runs when project is truthy, making the inner condition redundant.

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2b7ad36 and b73b66e.

📒 Files selected for processing (4)
  • backend/apps/github/graphql/queries/repository_contributor.py (1 hunks)
  • backend/apps/github/models/mixins/repository.py (1 hunks)
  • backend/apps/github/models/repository_contributor.py (2 hunks)
  • backend/apps/owasp/models/common.py (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • backend/apps/github/graphql/queries/repository_contributor.py
🧰 Additional context used
🧬 Code Graph Analysis (1)
backend/apps/owasp/models/common.py (1)
backend/apps/github/models/repository_contributor.py (2)
  • RepositoryContributor (14-191)
  • get_top_contributors (109-191)
⏰ Context from checks skipped due to timeout of 90000ms (4)
  • GitHub Check: Run frontend unit tests
  • GitHub Check: Run backend tests
  • GitHub Check: Run frontend e2e tests
  • GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (8)
backend/apps/github/models/repository_contributor.py (6)

4-5: Good addition of necessary imports for query functionality.

The new imports of F, Sum, and Window from django.db.models along with Rank from django.db.models.functions are appropriate for the new functionality being added to the class.


108-123: Well-documented class method with clear purpose and parameters.

This new class method provides a centralized implementation for retrieving top contributors, with good documentation that clearly explains the purpose, parameters, and return values.


124-139: Good base query construction with appropriate filters.

The initial queryset construction and repository/organization filtering looks well-implemented, using appropriate select_related calls to optimize database access.


145-160: Effective implementation of top contributors ranking logic.

The ranking implementation using Window functions is an efficient approach to get the top contributor per user when not filtering by repositories. Using the Rank window function with partitioning by user login ensures we get each user's best contribution.


161-174: Efficient aggregation and conditional annotations.

Good use of values() and annotate() to efficiently aggregate total contributions and conditionally add project information only when needed.


175-191: Clean result formatting with proper data transformation.

The method appropriately transforms the raw query result into a standardized format, including stripping the "www-project-" prefix from project keys when present.

backend/apps/github/models/mixins/repository.py (1)

102-104: Good refactoring to use centralized method.

The property has been simplified to use the new centralized get_top_contributors method, reducing code duplication and improving maintainability.

backend/apps/owasp/models/common.py (1)

196-198: Clean implementation using centralized method.

The method has been refactored to delegate to the new RepositoryContributor.get_top_contributors method, simplifying the code while maintaining the same functionality.

@samyak003 samyak003 marked this pull request as ready for review April 23, 2025 17:51
@samyak003 samyak003 marked this pull request as ready for review April 23, 2025 17:51
Copy link
Collaborator

@arkid15r arkid15r left a comment

Choose a reason for hiding this comment

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

We want to migrate all separate cases to a single endpoint supporting filtering for any entity -- entire organization, project, chapter, commitee, repository, etc.

Here are graphql queries to migrate - https://github.com/search?q=repo%3AOWASP%2FNest%20topContributors%20path%3A**%2Fqueries%2F**&type=code

@arkid15r arkid15r marked this pull request as draft April 27, 2025 01:43
@arkid15r
Copy link
Collaborator

@samyak003 are you still interested in finishing this task?

@samyak003
Copy link
Contributor Author

Yes, I'll complete this task by this weekend

@arkid15r
Copy link
Collaborator

@samyak003 I didn't see any promised updates 🤷‍♂️
Are you still interested in finishing this?

@samyak003 samyak003 marked this pull request as ready for review May 12, 2025 19:12
@samyak003 samyak003 requested a review from kasya as a code owner May 12, 2025 19:12
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

🔭 Outside diff range comments (1)
backend/apps/github/models/repository_contributor.py (1)

194-211: 💡 Verification agent

🧩 Analysis chain

Clean result formatting with prefix handling.

The result transformation cleanly formats the output dictionaries and handles the removal of the "www-project-" prefix from project keys. The use of get() with default values ensures the method handles cases where project information might not be present.

Consider adding a unit test to verify that this method behaves correctly with various combinations of filter parameters, particularly checking the project_key formatting.


🏁 Script executed:

#!/bin/bash
# Check if there are tests for the get_top_contributors method

echo "Searching for test files for RepositoryContributor model..."
fd "test.*repository_contributor.*\.py" --type f

echo "\nSearching for tests of the get_top_contributors method..."
rg "test.*get_top_contributors" --type py

Length of output: 352


Add missing unit tests for get_top_contributors

It looks like there aren’t any existing tests covering the get_top_contributors method in backend/apps/github/models/repository_contributor.py. Please add unit tests to ensure:

  • The method returns the correct number of contributors and respects the limit.
  • Contributions are ordered descending by total_contributions.
  • project_key formatting:
    • Returns None when project_key is absent.
    • Strips the "www-project-" prefix when present.
  • project_name, login, name, avatar_url, and contributions_count are all populated (or None where expected).
🧹 Nitpick comments (4)
frontend/src/app/chapters/[chapterKey]/page.tsx (1)

27-27: Consider adding a null check for topContributors data.

While the implementation correctly sets the topContributors state, there's no explicit check for data?.topContributors being undefined.

-      setTopContributors(data?.topContributors)
+      setTopContributors(data?.topContributors || [])
frontend/src/app/organizations/[organizationKey]/repositories/[repositoryKey]/page.tsx (1)

32-32: Consider adding a null check for topContributors data.

While the implementation correctly sets the topContributors state, there's no explicit check for data?.topContributors being undefined.

-      setTopContributors(data?.topContributors)
+      setTopContributors(data?.topContributors || [])
frontend/src/app/committees/[committeeKey]/page.tsx (1)

36-36: Consider adding a null check for topContributors data.

While the implementation correctly sets the topContributors state, there's no explicit check for data?.topContributors being undefined.

-      setTopContributors(data?.topContributors)
+      setTopContributors(data?.topContributors || [])
frontend/src/app/projects/[projectKey]/page.tsx (1)

34-34: Consider adding a null check for topContributors data.

While the implementation correctly sets the topContributors state, there's no explicit check for data?.topContributors being undefined.

-      setTopContributors(data?.topContributors)
+      setTopContributors(data?.topContributors || [])
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b73b66e and d719f24.

📒 Files selected for processing (10)
  • backend/apps/github/graphql/queries/repository_contributor.py (1 hunks)
  • backend/apps/github/models/repository_contributor.py (2 hunks)
  • frontend/src/app/chapters/[chapterKey]/page.tsx (4 hunks)
  • frontend/src/app/committees/[committeeKey]/page.tsx (3 hunks)
  • frontend/src/app/organizations/[organizationKey]/repositories/[repositoryKey]/page.tsx (3 hunks)
  • frontend/src/app/projects/[projectKey]/page.tsx (4 hunks)
  • frontend/src/server/queries/chapterQueries.ts (1 hunks)
  • frontend/src/server/queries/committeeQueries.ts (1 hunks)
  • frontend/src/server/queries/projectQueries.ts (1 hunks)
  • frontend/src/server/queries/repositoryQueries.ts (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
frontend/src/app/organizations/[organizationKey]/repositories/[repositoryKey]/page.tsx (2)
frontend/src/types/contributor.ts (1)
  • TopContributorsTypeGraphql (8-15)
frontend/src/server/queries/repositoryQueries.ts (1)
  • GET_REPOSITORY_DATA (3-56)
frontend/src/app/committees/[committeeKey]/page.tsx (2)
frontend/src/types/committee.ts (1)
  • CommitteeDetailsTypeGraphQL (21-35)
frontend/src/types/contributor.ts (1)
  • TopContributorsTypeGraphql (8-15)
frontend/src/app/chapters/[chapterKey]/page.tsx (1)
frontend/src/types/contributor.ts (1)
  • TopContributorsTypeGraphql (8-15)
frontend/src/app/projects/[projectKey]/page.tsx (1)
frontend/src/types/contributor.ts (1)
  • TopContributorsTypeGraphql (8-15)
backend/apps/github/models/repository_contributor.py (3)
backend/apps/owasp/models/common.py (1)
  • get_top_contributors (196-198)
backend/apps/github/models/repository.py (1)
  • project (150-152)
backend/apps/github/models/managers/repository_contributor.py (4)
  • by_humans (12-16)
  • by_humans (36-38)
  • to_community_repositories (18-20)
  • to_community_repositories (40-42)
🔇 Additional comments (26)
frontend/src/server/queries/projectQueries.ts (1)

62-67: Good refactoring of top contributors to root level query

Moving topContributors from being nested inside the project object to a separate top-level field with a project filter parameter is a solid architectural improvement. This change:

  1. Decouples contributor data from project data
  2. Enables more flexible data fetching
  3. Follows the pattern of centralizing contributor logic on the backend

The implementation correctly passes the project key as a parameter and maintains the same field selection.

frontend/src/server/queries/chapterQueries.ts (1)

20-25: Good consistent refactoring of top contributors

This change correctly follows the same pattern used in projectQueries.ts, moving topContributors to a root-level field with a chapter filter parameter. The consistent approach across different entity types improves maintainability and indicates a thoughtful architectural decision.

frontend/src/server/queries/repositoryQueries.ts (1)

49-54: Appropriate refactoring with dual parameter filtering

Moving topContributors to a root-level field with both repository and organization filters correctly implements the refactoring pattern. This maintains consistency with the other query files while appropriately handling the repository context which requires both repository and organization identifiers.

frontend/src/server/queries/committeeQueries.ts (1)

19-26: Enhanced contributor data with project information

This change follows the established refactoring pattern but extends the contributor fields to include projectKey and projectName. This enhancement provides valuable context when displaying committee contributors, associating them with their respective projects.

frontend/src/app/chapters/[chapterKey]/page.tsx (3)

8-8: Appropriate import for the refactored approach.

The import of TopContributorsTypeGraphql is correctly added to support the new state management approach for top contributors.


17-17: Good separation of concerns with new state variable.

Creating a dedicated state variable for top contributors properly decouples this data from the chapter object, aligning with the backend changes that now support filtering contributors by chapter.


70-70: Correctly updates prop to use the new state variable.

The component now correctly passes the dedicated topContributors state to the DetailsCard instead of accessing it through the chapter object.

frontend/src/app/organizations/[organizationKey]/repositories/[repositoryKey]/page.tsx (3)

15-15: Appropriate import for the refactored approach.

The import of TopContributorsTypeGraphql is correctly added to support the new state management approach for top contributors.


24-24: Good separation of concerns with new state variable.

Creating a dedicated state variable for top contributors properly decouples this data from the repository object, aligning with the backend changes that now support filtering by repository and organization.


115-115: Correctly updates prop to use the new state variable.

The component now correctly passes the dedicated topContributors state to the DetailsCard instead of accessing it through the repository object.

frontend/src/app/committees/[committeeKey]/page.tsx (3)

15-15: Appropriate import for the refactored approach.

The import of TopContributorsTypeGraphql is correctly added to support the new state management approach for top contributors.


23-23: Good separation of concerns with new state variable.

Creating a dedicated state variable for top contributors properly decouples this data from the committee object, aligning with the backend changes that now support filtering by committee.


91-91: Correctly updates prop to use the new state variable.

The component now correctly passes the dedicated topContributors state to the DetailsCard instead of accessing it through the committee object.

frontend/src/app/projects/[projectKey]/page.tsx (3)

14-14: Appropriate import for the refactored approach.

The import of TopContributorsTypeGraphql is correctly added to support the new state management approach for top contributors.


25-25: Good separation of concerns with new state variable.

Creating a dedicated state variable for top contributors properly decouples this data from the project object, aligning with the backend changes that now support filtering by project.


103-103: Correctly updates prop to use the new state variable.

The component now correctly passes the dedicated topContributors state to the DetailsCard instead of accessing it through the project object.

backend/apps/github/graphql/queries/repository_contributor.py (4)

17-20: Excellent extension of the filtering capabilities!

You've added new optional filter parameters (project, chapter, committee, repository) to the top_contributors GraphQL query. This allows for more flexible and precise querying of contributor data, which will enhance the usability of this API endpoint.


23-43: Good docstring updates and parameter handling.

The resolver method signature has been properly updated to include all the new filter parameters, and the docstring has been expanded with clear descriptions of each parameter. This makes the API well-documented and ensures future developers understand how to use these parameters.


49-56: Clean implementation using the centralized method.

The refactoring to use RepositoryContributor.get_top_contributors() centralizes the query logic and makes this resolver much cleaner. This approach will be more maintainable as it eliminates duplicate query logic across the codebase.


59-65: Simplified field mapping in the result transformation.

The mapping of query results to RepositoryContributorNode fields has been simplified by directly accessing the flattened dictionary keys returned by the new get_top_contributors method. This is more readable and less error-prone than handling nested dictionary keys.

backend/apps/github/models/repository_contributor.py (6)

4-5: Appropriate imports for advanced ORM functionality.

Adding imports for F, Sum, Window from django.db.models and Rank from django.db.models.functions allows for efficient in-database aggregation and ranking operations, which are essential for the get_top_contributors method.


108-133: Well-documented method signature.

The class method get_top_contributors has a comprehensive docstring that clearly explains the purpose and all parameters. This is excellent practice for maintainability and will help other developers understand how to use this method.


134-138: Efficient base queryset construction.

Starting with a clean queryset that filters to human contributors, community repositories, and uses select_related for related models is an excellent approach for performance optimization.


143-162: Consistent filtering patterns for different entity types.

The filtering logic consistently uses iexact for case-insensitive matching and properly handles the prefix conventions (e.g., "www-project-") for project, chapter, and committee keys. This ensures the API is user-friendly while maintaining compatibility with the database's naming conventions.


164-179: Sophisticated ranking logic for non-repository-specific queries.

Using window functions to rank contributors by contribution count and filtering to the top contributor per user is an efficient approach. This ensures that users don't appear multiple times in the results when querying across repositories, which provides a cleaner user experience.


181-193: Smart conditional project information inclusion.

Only including project information when not already filtering by project, organization, or repositories avoids redundancy and ensures the response contains relevant information based on the query context.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
backend/apps/github/models/repository_contributor.py (1)

159-171: Consider adding a comment explaining the rank filtering logic.

The Window function with Rank is a powerful feature, but its purpose might not be immediately clear to all developers. Consider adding a comment explaining that this is used to select only the top contribution from each user across multiple repositories.

# Project contributors only for main/project/organization pages.
if not (chapter or committee or repository):
    queryset = (
        queryset.filter(repository__project__isnull=False)
        .annotate(
            rank=Window(
                expression=Rank(),
                order_by=F("contributions_count").desc(),
                partition_by=F("user__login"),
            )
        )
+       # Only include the user's top contribution repository to avoid counting the same user multiple times
        .filter(rank=1)
    )
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d719f24 and e9b917c.

📒 Files selected for processing (15)
  • backend/apps/github/graphql/queries/repository_contributor.py (1 hunks)
  • backend/apps/github/models/mixins/repository.py (1 hunks)
  • backend/apps/github/models/repository_contributor.py (2 hunks)
  • backend/apps/owasp/models/common.py (0 hunks)
  • backend/apps/owasp/models/mixins/chapter.py (2 hunks)
  • backend/apps/owasp/models/mixins/committee.py (2 hunks)
  • backend/apps/owasp/models/mixins/project.py (2 hunks)
  • frontend/__tests__/e2e/pages/ChapterDetails.spec.ts (1 hunks)
  • frontend/__tests__/unit/data/mockChapterDetailsData.ts (1 hunks)
  • frontend/__tests__/unit/data/mockCommitteeDetailsData.ts (1 hunks)
  • frontend/__tests__/unit/data/mockProjectDetailsData.ts (1 hunks)
  • frontend/__tests__/unit/data/mockRepositoryData.ts (1 hunks)
  • frontend/__tests__/unit/pages/ChapterDetails.test.tsx (3 hunks)
  • frontend/__tests__/unit/pages/CommitteeDetails.test.tsx (2 hunks)
  • frontend/src/app/committees/[committeeKey]/page.tsx (2 hunks)
💤 Files with no reviewable changes (1)
  • backend/apps/owasp/models/common.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • frontend/src/app/committees/[committeeKey]/page.tsx
  • backend/apps/github/models/mixins/repository.py
🧰 Additional context used
🧬 Code Graph Analysis (6)
frontend/__tests__/e2e/pages/ChapterDetails.spec.ts (1)
frontend/__tests__/unit/data/mockChapterDetailsData.ts (1)
  • mockChapterDetailsData (1-36)
frontend/__tests__/unit/pages/ChapterDetails.test.tsx (1)
frontend/__tests__/unit/data/mockChapterDetailsData.ts (1)
  • mockChapterDetailsData (1-36)
backend/apps/owasp/models/mixins/committee.py (1)
backend/apps/github/models/repository_contributor.py (2)
  • RepositoryContributor (12-200)
  • get_top_contributors (113-200)
backend/apps/owasp/models/mixins/chapter.py (1)
backend/apps/github/models/repository_contributor.py (2)
  • RepositoryContributor (12-200)
  • get_top_contributors (113-200)
frontend/__tests__/unit/pages/CommitteeDetails.test.tsx (1)
frontend/__tests__/unit/data/mockCommitteeDetailsData.ts (1)
  • mockCommitteeDetailsData (1-24)
backend/apps/owasp/models/mixins/project.py (2)
backend/apps/github/models/repository_contributor.py (2)
  • RepositoryContributor (12-200)
  • get_top_contributors (113-200)
backend/apps/github/models/repository.py (1)
  • project (152-154)
⏰ Context from checks skipped due to timeout of 90000ms (4)
  • GitHub Check: Run frontend unit tests
  • GitHub Check: Run backend tests
  • GitHub Check: CodeQL (python)
  • GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (27)
frontend/__tests__/unit/data/mockCommitteeDetailsData.ts (1)

10-23: LGTM! Data structure refactor correctly decouples contributors from committee entity.

The change correctly implements the refactored data structure where topContributors is now a top-level property rather than nested inside the committee object. This aligns with the broader architectural changes to query top contributors as a separate entity across the application.

frontend/__tests__/e2e/pages/ChapterDetails.spec.ts (1)

9-9: LGTM! Mock JSON structure correctly updated to match refactored GraphQL response.

The mock response structure has been updated to directly use mockChapterDetailsData as the data property, which aligns with the changes in the GraphQL query structure where topContributors is now queried as a separate root-level field.

frontend/__tests__/unit/pages/ChapterDetails.test.tsx (3)

36-36: LGTM! Updated mock data structure for useQuery hook.

The test mock correctly reflects the refactored GraphQL query response structure where the data is no longer nested inside a chapter property.


59-59: LGTM! Consistent mock data structure for useQuery hook.

The change maintains consistency with the updated data structure pattern across all test cases.


112-112: LGTM! Consistent mock data structure for incomplete contributors test case.

The test case for handling incomplete contributor data correctly follows the same pattern of directly using the data object without extra nesting.

frontend/__tests__/unit/data/mockProjectDetailsData.ts (1)

94-99: LGTM! Top contributors moved to top-level property.

The topContributors array has been correctly moved from being nested inside the project object to being a top-level property in the exported mockProjectDetailsData object. This aligns with the broader refactoring pattern applied consistently across the codebase.

frontend/__tests__/unit/data/mockRepositoryData.ts (1)

63-68: LGTM: Mock data structure updated to reflect architectural changes

The refactoring of the topContributors array from a nested property inside the repository object to a top-level property matches the structural changes made to the GraphQL queries. This properly aligns the mock data with how top contributors are now managed as separate state in the React components.

backend/apps/owasp/models/mixins/committee.py (2)

5-5: Import of RepositoryContributor added correctly

This import enables the use of the centralized get_top_contributors class method.


30-30: Good refactoring: Centralized top contributors retrieval

The property now uses the centralized RepositoryContributor.get_top_contributors class method with the committee key as a filter parameter, which aligns with the architectural change to standardize contributor retrieval across the application.

frontend/__tests__/unit/pages/CommitteeDetails.test.tsx (3)

29-29: Test mock updated to use new data structure

The mock data structure now correctly reflects the updated GraphQL query response shape where data is provided directly rather than nested under a committee property.


84-94: Test fixture correctly restructured

The committeeDataWithIncompleteContributors object has been properly restructured to have committee and topContributors as sibling properties at the top level, which matches the new GraphQL response format.


96-96: Test mock updated to match new data structure

The useQuery mock now returns the entire data object directly, matching how the actual component would receive GraphQL data after the refactoring.

backend/apps/owasp/models/mixins/chapter.py (3)

3-4: Future annotations import added

The __future__.annotations import allows for using forward references in type annotations, which is good practice in Python files with circular dependencies.


5-5: Import of RepositoryContributor added correctly

This import enables the use of the centralized get_top_contributors class method.


74-74: Good refactoring: Centralized top contributors retrieval

Similar to the committee mixin, this property now uses the centralized RepositoryContributor.get_top_contributors class method with the chapter key as a filter parameter, streamlining how contributors are fetched across different entity types.

backend/apps/owasp/models/mixins/project.py (2)

6-6: New import looks good.

The import of RepositoryContributor from apps.github.models.repository_contributor is correctly added to support the updated implementation.


105-108: Implementation refactored to use centralized method.

The idx_top_contributors property now uses the centralized RepositoryContributor.get_top_contributors class method instead of a local implementation. This is a good refactoring that promotes code reuse and consistency.

frontend/__tests__/unit/data/mockChapterDetailsData.ts (1)

1-29: Test data structure aligned with updated GraphQL schema.

The mock data structure has been updated to nest chapter-specific properties under a chapter object, reflecting changes in the GraphQL schema. This structure change properly mimics how the data will be returned from the API.

backend/apps/github/graphql/queries/repository_contributor.py (3)

15-23: Good enhancement of filter options for the top_contributors query.

Adding optional filter parameters for chapter, committee, project, and repository improves the flexibility of the API and allows clients to retrieve more specific data.


25-51: Well-documented resolver with clear parameter descriptions.

The resolver method signature and docstring have been properly updated to reflect the new parameters, providing clear documentation for future developers.


52-71: Simplified resolver implementation using centralized model method.

The resolver now delegates to the centralized RepositoryContributor.get_top_contributors method instead of containing complex query logic. This is a good refactoring that separates concerns and makes the code more maintainable.

backend/apps/github/models/repository_contributor.py (6)

4-5: Added necessary imports for the new method.

The imports for F, Sum, and Window from django.db.models and Rank from django.db.models.functions are correctly added to support the new get_top_contributors method.


112-135: Well-structured method signature and documentation.

The new get_top_contributors class method has a clear signature with appropriate default values and comprehensive docstring documentation for all parameters.


136-171: Robust filtering logic with appropriate context-based handling.

The implementation properly builds the queryset with appropriate filters based on the provided parameters. The special handling for project contributors when no chapter, committee, or repository filter is applied is a thoughtful design choice.


172-186: Efficient aggregation and ordering of contributors.

The query effectively aggregates contributions by user and includes relevant project information, with proper ordering and limit application.


188-200: Clean data transformation for API consumption.

The method returns a well-structured list of dictionaries with all necessary contributor information, correctly handling the project key transformation.


112-200: Comprehensive centralized implementation that improves code organization.

The new get_top_contributors method successfully centralizes the logic for retrieving top contributors, which was previously scattered across multiple places. This refactoring improves maintainability, reduces code duplication, and provides a consistent interface for different parts of the application.

@arkid15r arkid15r disabled auto-merge May 14, 2025 16:27
@arkid15r arkid15r enabled auto-merge May 14, 2025 16:27
@sonarqubecloud
Copy link

@arkid15r arkid15r added this pull request to the merge queue May 14, 2025
Merged via the queue into OWASP:main with commit e2712b9 May 14, 2025
23 checks passed
@coderabbitai coderabbitai bot mentioned this pull request May 14, 2025
1 task
shdwcodr pushed a commit to shdwcodr/Nest that referenced this pull request Jun 5, 2025
* Added few changes to repository_contributor file

* Refactored the code

* Added few changes

* Update code

* Refactor code

* Fix tests

* Update code

---------

Co-authored-by: Arkadii Yakovets <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unify top contributors logic

2 participants