Skip to content

Extend user model with total contributions field. #1209

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

Merged
merged 25 commits into from
Apr 20, 2025

Conversation

ahmedxgouda
Copy link
Collaborator

@ahmedxgouda ahmedxgouda commented Mar 30, 2025

Resolves #1192

  • Backend implementation
  • Frontend implementation
  • Backend tests
  • Frontend tests

Copy link
Contributor

coderabbitai bot commented Mar 30, 2025

Summary by CodeRabbit

  • New Features
    • User profiles now display a "Contributions" count, reflecting each user's total contributions.
  • Improvements
    • User statistics section updated to show contributions instead of issues and releases.
    • Placeholder text for missing user details changed from "Not provided" to "N/A".
  • Bug Fixes
    • Tests updated to verify the display of the contributions count and new placeholder text.
  • Chores
    • Backend processes updated to efficiently calculate and store user contributions.
    • New management command added for updating user contributions in bulk.
      """

Summary by CodeRabbit

  • New Features

    • User profiles now display a "Contributions" count, showing the total number of contributions made by each user.
    • The user statistics section has been updated to highlight contributions instead of issues and releases.
  • Bug Fixes

    • Placeholder text for missing user details is now shown as "N/A" instead of "Not provided".
  • Chores

    • Added a backend command to update users' contributions counts in bulk.
    • Updated backend processes to support and migrate the new contributions count field.
  • Tests

    • Enhanced tests to verify the correct display and handling of the contributions count.

Walkthrough

This change introduces a new contributions_count field to the backend User model, including database migrations, model updates, and a management command for batch-populating this field based on aggregated repository contributions. The indexing mixin is updated to use the new field directly. The GraphQL schema and frontend type definitions are extended to expose and consume this field. The user details page UI and related tests are updated to display the contributions count in the statistics section, replacing previous statistics related to issues and releases. All relevant tests and mock data are updated accordingly.

Changes

Files/Paths Change Summary
backend/apps/github/models/user.py
backend/apps/github/migrations/0021_user_contributions_count.py
backend/apps/github/migrations/0022_merge_20250417_1000.py
backend/apps/github/migrations/0023_alter_user_contributions_count.py
Add contributions_count field to User model (initially as IntegerField, then as PositiveIntegerField via migration); add merge migration.
backend/apps/github/models/mixins/user.py Update indexing mixin to return the new contributions_count field instead of recalculating it dynamically.
backend/apps/github/management/commands/github_update_users.py New management command to recalculate and populate contributions_count for all users in batches.
backend/apps/github/graphql/nodes/user.py Expose contributions_count in the GraphQL UserNode schema.
backend/tests/apps/github/graphql/nodes/user_test.py Update tests to check for the presence of contributions_count in the GraphQL node.
backend/Makefile Add target to run the new management command; update data update process to include this step.
frontend/src/types/user.ts Add contributions_count to user-related types and props.
frontend/src/server/queries/userQueries.ts Extend user GraphQL query to request contributionsCount.
frontend/src/app/members/[memberKey]/page.tsx Update user details UI: display contributions_count in statistics, remove issues/releases, update fallback text.
frontend/tests/e2e/pages/UserDetails.spec.ts
frontend/tests/unit/pages/UserDetails.test.tsx
frontend/tests/unit/data/mockUserDetails.ts
Update and extend tests and mock data to verify correct display and handling of contributions_count in the UI.

Assessment against linked issues

Objective Addressed Explanation
Add contributions_count field to the User model with proper migration (#1192)
Populate contributions_count during the data sync process using the correct aggregation logic (#1192)
Update indexing mixin to use the pre-calculated field instead of recalculating dynamically (#1192)
Update user details page to display contributions_count in the statistics section, replacing issues/releases (#1192)
Update/add tests for the new functionality and UI (#1192)

Suggested reviewers

  • kasya
    """

📜 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 66c1723 and e087f58.

📒 Files selected for processing (1)
  • backend/apps/github/management/commands/github_update_users.py (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • backend/apps/github/management/commands/github_update_users.py
⏰ Context from checks skipped due to timeout of 90000ms (4)
  • GitHub Check: Run frontend unit tests
  • GitHub Check: Run frontend e2e tests
  • GitHub Check: Run backend tests
  • GitHub Check: CodeQL (javascript-typescript)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

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

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

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

Other keywords and placeholders

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

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
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 (2)
backend/apps/github/models/user.py (2)

85-96: Consider moving contribution calculation to a separate method.

The implementation correctly calculates the total contributions by summing the contributions_count from all repository contributions for the user. However, this logic could be better encapsulated in a separate method to improve code organization and maintainability.

Consider refactoring the code like this:

- if does_exist:
-     from apps.github.models.repository_contributor import RepositoryContributor
- 
-     contributions_count = (
-         RepositoryContributor.objects.by_humans()
-         .filter(user=user)
-         .aggregate(total_contributions=Sum("contributions_count"))[
-             "total_contributions"
-         ]
-         or 0
-     )
-     user.contributions_count = contributions_count
+ if does_exist:
+     user.update_contributions_count()

And add this method to the User class:

def update_contributions_count(self):
    """Update the user's total contributions count."""
    from apps.github.models.repository_contributor import RepositoryContributor
    
    self.contributions_count = (
        RepositoryContributor.objects.by_humans()
        .filter(user=self)
        .aggregate(total_contributions=Sum("contributions_count"))[
            "total_contributions"
        ]
        or 0
    )

86-87: Consider moving import to the module level.

Importing RepositoryContributor inside the method prevents potential circular imports but could be moved to the top of the file for better code organization, as it's a standard practice in Python.

from django.db import models
from django.db.models import Sum

from apps.common.models import TimestampedModel
from apps.github.constants import GITHUB_GHOST_USER_LOGIN, OWASP_FOUNDATION_LOGIN
from apps.github.models.common import GenericUserModel, NodeModel
from apps.github.models.mixins.user import UserIndexMixin
from apps.github.models.organization import Organization
+from apps.github.models.repository_contributor import RepositoryContributor
📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 90e8cd5 and 68effca.

📒 Files selected for processing (7)
  • backend/apps/github/graphql/nodes/user.py (3 hunks)
  • backend/apps/github/migrations/0020_user_contributions_count.py (1 hunks)
  • backend/apps/github/models/mixins/user.py (1 hunks)
  • backend/apps/github/models/user.py (3 hunks)
  • frontend/src/api/queries/userQueries.ts (1 hunks)
  • frontend/src/pages/UserDetails.tsx (2 hunks)
  • frontend/src/types/user.ts (3 hunks)
🧰 Additional context used
🧬 Code Definitions (3)
frontend/src/pages/UserDetails.tsx (1)
frontend/src/types/user.ts (1)
  • user (3-18)
backend/apps/github/models/user.py (2)
backend/apps/github/models/repository_contributor.py (2)
  • from_github (43-53)
  • RepositoryContributor (12-78)
backend/apps/github/models/managers/repository_contributor.py (2)
  • by_humans (12-16)
  • by_humans (36-38)
backend/apps/github/graphql/nodes/user.py (1)
backend/apps/github/models/mixins/user.py (1)
  • idx_contributions_count (107-109)
⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: Run frontend unit tests
  • GitHub Check: Run frontend e2e tests
🔇 Additional comments (15)
frontend/src/api/queries/userQueries.ts (1)

48-48: Good addition to the user query!

Adding the contributionsCount field to the GraphQL query allows the frontend to display this important statistic for users, enhancing the user profile data shown in the UI.

backend/apps/github/migrations/0020_user_contributions_count.py (1)

11-16: Migration looks good for adding the contributions count field.

The migration correctly adds an IntegerField with a default value of 0 and appropriate verbose name. This matches the PR's objective to extend the user model with a total contributions field.

frontend/src/pages/UserDetails.tsx (2)

9-9: Good import of the code merge icon for contributions.

The faCodeMerge icon is semantically appropriate for representing user contributions.


218-222: Well-implemented contributions count display.

The implementation follows the existing pattern for user stats, correctly using millify for formatting large numbers and pluralize for grammatical correctness. This enhances the user profile with valuable contribution information.

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

107-109:

✅ Verification successful

Performance improvement by directly returning stored value.

This change simplifies the code and improves performance by directly returning the stored contributions_count field instead of calculating it on-the-fly with database queries. However, ensure that there's a mechanism elsewhere in the codebase to properly maintain and update this field when a user's contributions change.

To verify that the contributions_count field is properly updated elsewhere in the codebase, run:


🏁 Script executed:

#!/bin/bash
# Search for code that updates the contributions_count field
rg -A 2 -B 2 "contributions_count\s*=" --type py

Length of output: 3398


Update Confirmed: Contributions Count Field Maintenance Verified

The code now directly returns the stored contributions_count value, providing a clear performance benefit. Our investigation confirms that the field is properly maintained elsewhere in the codebase. Specifically:

  • In backend/apps/github/models/user.py, the contributions_count field is updated (e.g., via the assignment user.contributions_count = contributions_count followed by user.save()).
  • Tests in backend/tests/apps/github/models/repository_contributor_test.py verify that the contributions count is correctly maintained.

No further changes are required.

backend/apps/github/models/user.py (4)

4-4: Good addition of Sum import for aggregation.

The import of Sum from django.db.models is appropriate for calculating the total contributions across repositories.


26-26: LGTM: New contributions_count field added properly.

The contributions_count field is correctly defined as an IntegerField with a default value of 0, which is appropriate for tracking user contributions.


73-76: Good explanation through comments.

The comments clearly explain why you're creating the does_exist flag, which helps with understanding the implementation approach.


79-79: Properly setting the existence flag.

Setting does_exist to True when a user is found in the database is correct and aligns with the implementation design.

frontend/src/types/user.ts (3)

17-17: LGTM: Properly added contributions_count to user type.

The contributions_count property is correctly added to the user type with the appropriate type of number.


61-61: LGTM: Properly added contributions_count to User type.

The contributions_count property is correctly added to the User type with the appropriate type of number.


82-82: LGTM: Properly added contributionsCount to UserDetailsProps interface.

The contributionsCount property is correctly added to the UserDetailsProps interface with the appropriate type of number. The naming follows the camelCase convention used for other properties in this interface.

backend/apps/github/graphql/nodes/user.py (3)

47-47: LGTM: Properly added contributions_count field to UserNode.

The contributions_count field is correctly defined as a graphene.Int() type, which is appropriate for exposing the contributions count in the GraphQL schema.


63-63: LGTM: Properly added contributions_count to fields tuple.

The contributions_count field is correctly added to the fields tuple in the Meta class, which makes it available for GraphQL queries.


94-96: LGTM: Proper implementation of resolve_contributions_count.

The resolver method correctly returns self.idx_contributions_count, which (based on the relevant code snippet) directly returns the contributions_count value from the user model.

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

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 68effca and 798dbf5.

📒 Files selected for processing (5)
  • backend/tests/apps/github/graphql/nodes/user_test.py (1 hunks)
  • backend/tests/apps/github/models/user_test.py (1 hunks)
  • frontend/__tests__/e2e/pages/UserDetails.spec.ts (1 hunks)
  • frontend/__tests__/unit/data/mockUserDetails.ts (1 hunks)
  • frontend/__tests__/unit/pages/UserDetails.test.tsx (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
  • GitHub Check: Run frontend e2e tests
  • GitHub Check: Run frontend unit tests
  • GitHub Check: Run backend tests
  • GitHub Check: CodeQL (python)
  • GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (5)
frontend/__tests__/unit/data/mockUserDetails.ts (1)

15-15: LGTM: Properly added the new contributions count field to the mock data.

The addition of contributionsCount: 100 to the mock user data correctly aligns with the new field being added to the user model. This change ensures that tests using this mock data will properly verify the display of contribution counts in the UI.

backend/tests/apps/github/graphql/nodes/user_test.py (1)

37-37: LGTM: Test properly updated to include the new field.

Adding "contributions_count" to the expected fields ensures that the test verifies this new field is properly exposed through the GraphQL API. This is an appropriate and necessary test update.

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

30-30: LGTM: E2E test correctly validates the new contributions count UI element.

The added assertion ensures that the text '100 Contributions' is visible on the page, validating that the UI properly displays the user's contribution count. This is a good addition to the test coverage.

frontend/__tests__/unit/pages/UserDetails.test.tsx (1)

176-178: LGTM: Unit test properly verifies the contributions count display.

The added assertions correctly validate that the contributions count is displayed in the statistics section of the user details page. This complements the E2E test and ensures proper test coverage for the new feature.

backend/tests/apps/github/models/user_test.py (1)

35-35: Good addition of RepositoryContributor.objects mock.

This correctly adds a mock for the RepositoryContributor objects manager, which aligns with the PR's objective of adding contribution tracking functionality to the User model.

@patch("apps.github.models.user.User.objects.get")
@patch("apps.github.models.user.User.save")
def test_update_data(self, mock_save, mock_get):
def test_update_data(self, mock_save, mock_get, mock_repo_contributor):
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Test should utilize the mock_repo_contributor parameter.

While you've correctly added the parameter corresponding to the new mock, the test doesn't actually use this mock anywhere in its implementation. For proper test coverage of the new contribution counting functionality, you should:

  1. Set up appropriate return values for the mock
  2. Add assertions that verify the contributions_count is updated correctly
def test_update_data(self, mock_save, mock_get, mock_repo_contributor):
    gh_user_mock = Mock()
    gh_user_mock.raw_data = {"node_id": "12345"}
    gh_user_mock.bio = "Bio"
    gh_user_mock.hireable = True
    gh_user_mock.twitter_username = "twitter"

    mock_user = Mock(spec=User)
    mock_user.node_id = "12345"
    mock_get.return_value = mock_user

+   # Set up mock for repository contributor
+   mock_repo_contributor.filter.return_value.count.return_value = 5

    updated_user = User.update_data(gh_user_mock, save=True)

    mock_get.assert_called_once_with(node_id="12345")
    assert updated_user.node_id == "12345"
+   # Verify the contributions count is updated
+   mock_repo_contributor.filter.assert_called_once_with(user=mock_user)
+   assert updated_user.contributions_count == 5
📝 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.

Suggested change
def test_update_data(self, mock_save, mock_get, mock_repo_contributor):
def test_update_data(self, mock_save, mock_get, mock_repo_contributor):
gh_user_mock = Mock()
gh_user_mock.raw_data = {"node_id": "12345"}
gh_user_mock.bio = "Bio"
gh_user_mock.hireable = True
gh_user_mock.twitter_username = "twitter"
mock_user = Mock(spec=User)
mock_user.node_id = "12345"
mock_get.return_value = mock_user
# Set up mock for repository contributor
mock_repo_contributor.filter.return_value.count.return_value = 5
updated_user = User.update_data(gh_user_mock, save=True)
mock_get.assert_called_once_with(node_id="12345")
assert updated_user.node_id == "12345"
# Verify the contributions count is updated
mock_repo_contributor.filter.assert_called_once_with(user=mock_user)
assert updated_user.contributions_count == 5

@ahmedxgouda ahmedxgouda marked this pull request as ready for review March 30, 2025 07:50
@ahmedxgouda
Copy link
Collaborator Author

Hello @arkid15r . It's been a week :). Could you please review it?

@ahmedxgouda
Copy link
Collaborator Author

Hello @arkid15r . It's been a week :). Could you please review it?

I couldn't merge the migrations because of the permission error as I told you.

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

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 69b042d and ac783a3.

📒 Files selected for processing (5)
  • frontend/__tests__/unit/data/mockUserDetails.ts (1 hunks)
  • frontend/__tests__/unit/pages/UserDetails.test.tsx (1 hunks)
  • frontend/src/app/community/users/[userKey]/page.tsx (2 hunks)
  • frontend/src/server/queries/userQueries.ts (1 hunks)
  • frontend/src/types/user.ts (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • frontend/src/types/user.ts
  • frontend/tests/unit/pages/UserDetails.test.tsx
  • frontend/tests/unit/data/mockUserDetails.ts
🧰 Additional context used
🧬 Code Definitions (1)
frontend/src/app/community/users/[userKey]/page.tsx (1)
frontend/src/types/user.ts (1)
  • user (3-18)
🪛 GitHub Actions: Run CI/CD
frontend/src/app/community/users/[userKey]/page.tsx

[error] 203-203: Unstaged changes detected. Run make check and use git add to address it.

🔇 Additional comments (3)
frontend/src/server/queries/userQueries.ts (1)

49-49: Addition of contributionsCount field looks good!

The field is properly placed within the user query section and will enable the frontend to display the user's total contribution count. This aligns with the PR objective of extending the user model with a total contributions field.

frontend/src/app/community/users/[userKey]/page.tsx (2)

9-9: Import of faCodeMerge icon is appropriate.

The icon choice is well-suited for representing user contributions in the UI.


195-207: Implementation of the contributions count in userStats is well done.

The implementation correctly follows the established pattern of other user statistics, displaying the count with an appropriate icon and label. The optional chaining with fallback to 0 ensures the UI handles cases where the data might be undefined.

🧰 Tools
🪛 GitHub Actions: Run CI/CD

[error] 203-203: Unstaged changes detected. Run make check and use git add to address it.

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 (2)
backend/apps/github/migrations/0021_user_contributions_count.py (2)

9-9: Consider using double quotes for consistency.

The static analysis tool suggests using double quotes instead of single quotes to maintain consistency with the codebase style guide.

-        ('github', '0020_repositorycontributor_user_contrib_idx'),
+        ("github", "0020_repositorycontributor_user_contrib_idx"),
🧰 Tools
🪛 Ruff (0.8.2)

9-9: Single quotes found but double quotes preferred

Replace single quotes with double quotes

(Q000)


9-9: Single quotes found but double quotes preferred

Replace single quotes with double quotes

(Q000)


14-16: Consider using double quotes for string literals.

For consistency with the codebase style guide, consider using double quotes for these string literals.

-            model_name='user',
-            name='contributions_count',
-            field=models.IntegerField(default=0, verbose_name='Contributions count'),
+            model_name="user",
+            name="contributions_count",
+            field=models.IntegerField(default=0, verbose_name="Contributions count"),
🧰 Tools
🪛 Ruff (0.8.2)

14-14: Single quotes found but double quotes preferred

Replace single quotes with double quotes

(Q000)


15-15: Single quotes found but double quotes preferred

Replace single quotes with double quotes

(Q000)


16-16: Single quotes found but double quotes preferred

Replace single quotes with double quotes

(Q000)

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between ac783a3 and 0bb56d3.

📒 Files selected for processing (2)
  • backend/apps/github/migrations/0021_user_contributions_count.py (1 hunks)
  • frontend/src/app/community/users/[userKey]/page.tsx (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • frontend/src/app/community/users/[userKey]/page.tsx
🧰 Additional context used
🪛 Ruff (0.8.2)
backend/apps/github/migrations/0021_user_contributions_count.py

9-9: Single quotes found but double quotes preferred

Replace single quotes with double quotes

(Q000)


9-9: Single quotes found but double quotes preferred

Replace single quotes with double quotes

(Q000)


14-14: Single quotes found but double quotes preferred

Replace single quotes with double quotes

(Q000)


15-15: Single quotes found but double quotes preferred

Replace single quotes with double quotes

(Q000)


16-16: Single quotes found but double quotes preferred

Replace single quotes with double quotes

(Q000)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (1)
backend/apps/github/migrations/0021_user_contributions_count.py (1)

1-18: LGTM! Migration adds contributions_count field to User model.

The migration adds an IntegerField with default value 0 to track user contributions, which aligns with the PR objective of extending the user model with a total contributions field.

🧰 Tools
🪛 Ruff (0.8.2)

9-9: Single quotes found but double quotes preferred

Replace single quotes with double quotes

(Q000)


9-9: Single quotes found but double quotes preferred

Replace single quotes with double quotes

(Q000)


14-14: Single quotes found but double quotes preferred

Replace single quotes with double quotes

(Q000)


15-15: Single quotes found but double quotes preferred

Replace single quotes with double quotes

(Q000)


16-16: Single quotes found but double quotes preferred

Replace single quotes with double quotes

(Q000)

@ahmedxgouda
Copy link
Collaborator Author

Hello @arkid15r . It's been a week :). Could you please review it?

I couldn't merge the migrations because of the permission error as I told you.

Problem addressed!

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/migrations/0022_merge_20250417_1000.py (1)

9-10:

❓ Verification inconclusive

Prefer double quotes for consistency
Ruff and the pre-commit hook flag single-quoted strings here. Updating to double quotes will align with project style and resolve formatting errors.

Apply this diff:

-        ('github', '0021_release_release_published_at_idx'),
-        ('github', '0021_user_contributions_count'),
+        ("github", "0021_release_release_published_at_idx"),
+        ("github", "0021_user_contributions_count"),

Afterward, run:

to ensure all formatting checks pass.


🏁 Script executed:

pre-commit run --all-files

Length of output: 129


Replace single quotes with double quotes for consistency

It looks like your project enforces double‑quoted strings via Ruff/pre‑commit. Please update the tuple entries in backend/apps/github/migrations/0022_merge_20250417_1000.py:

-        ('github', '0021_release_release_published_at_idx'),
-        ('github', '0021_user_contributions_count'),
+        ("github", "0021_release_release_published_at_idx"),
+        ("github", "0021_user_contributions_count"),

After applying, run your formatting checks again (e.g. pre-commit run --all-files or ruff .) to confirm no lint errors remain.

🧰 Tools
🪛 Ruff (0.8.2)

9-9: Single quotes found but double quotes preferred

Replace single quotes with double quotes

(Q000)


9-9: Single quotes found but double quotes preferred

Replace single quotes with double quotes

(Q000)


10-10: Single quotes found but double quotes preferred

Replace single quotes with double quotes

(Q000)


10-10: Single quotes found but double quotes preferred

Replace single quotes with double quotes

(Q000)

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between e94b231 and 0e67617.

📒 Files selected for processing (1)
  • backend/apps/github/migrations/0022_merge_20250417_1000.py (1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
backend/apps/github/migrations/0022_merge_20250417_1000.py

9-9: Single quotes found but double quotes preferred

Replace single quotes with double quotes

(Q000)


9-9: Single quotes found but double quotes preferred

Replace single quotes with double quotes

(Q000)


10-10: Single quotes found but double quotes preferred

Replace single quotes with double quotes

(Q000)


10-10: Single quotes found but double quotes preferred

Replace single quotes with double quotes

(Q000)

🪛 GitHub Actions: Run CI/CD
backend/apps/github/migrations/0022_merge_20250417_1000.py

[error] 4-14: Code formatting issues fixed by pre-commit hook. Please run 'pre-commit run --all-files' locally to apply these changes.

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (3)
backend/apps/github/migrations/0022_merge_20250417_1000.py (3)

1-2: The migration header is auto-generated by Django and does not require manual edits.


3-3: Importing migrations from Django is correct and follows conventions.


6-14: Merge migration setup is correct
The Migration class properly depends on both 0021_release_release_published_at_idx and 0021_user_contributions_count, and has an empty operations list as expected for a merge migration.

🧰 Tools
🪛 Ruff (0.8.2)

9-9: Single quotes found but double quotes preferred

Replace single quotes with double quotes

(Q000)


9-9: Single quotes found but double quotes preferred

Replace single quotes with double quotes

(Q000)


10-10: Single quotes found but double quotes preferred

Replace single quotes with double quotes

(Q000)


10-10: Single quotes found but double quotes preferred

Replace single quotes with double quotes

(Q000)

🪛 GitHub Actions: Run CI/CD

[error] 4-14: Code formatting issues fixed by pre-commit hook. Please run 'pre-commit run --all-files' locally to apply these changes.

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: 2

🧹 Nitpick comments (3)
backend/apps/github/management/commands/github_update_users.py (3)

35-41: Consider using pagination to improve memory usage.

The current approach loads all users at once with User.objects.order_by("-created_at"), which could be memory-intensive for a large number of users. Consider using Django's pagination to process users in chunks.

-        active_users = User.objects.order_by("-created_at")
-        active_users_count = active_users.count()
-        users = []
-        for idx, user in enumerate(active_users[offset:]):
-            prefix = f"{idx + offset + 1} of {active_users_count - offset}"
+        active_users_queryset = User.objects.order_by("-created_at")
+        active_users_count = active_users_queryset.count()
+        users = []
+        batch_size = 1000
+        
+        # Process in chunks to avoid loading all users in memory
+        for idx, user in enumerate(active_users_queryset[offset:].iterator(chunk_size=batch_size)):
+            prefix = f"{idx + offset + 1} of {active_users_count - offset}"

35-57: Consider adding a transaction for atomicity.

To ensure database consistency, consider wrapping the main processing logic in a transaction, especially if this command might be interrupted or needs to be retried.

+from django.db import transaction

    def handle(self, *args, **options):
        """Handle the command execution.

        Args:
            *args: Variable length argument list.
            **options: Arbitrary keyword arguments containing command options.

        """
        offset = options["offset"]
        active_users = User.objects.order_by("-created_at")
        active_users_count = active_users.count()
        users = []
        
+        @transaction.atomic
+        def process_batch(batch_users):
+            User.bulk_save(batch_users, fields=("contributions_count",))
+            logger.info(f"Saved batch of {len(batch_users)} users")

        for idx, user in enumerate(active_users[offset:]):
            # ... existing code ...
            
            if not len(users) % 1000:
+                process_batch(users)
-                User.bulk_save(users, fields=("contributions_count",))
+                users = []

+        if users:
+            process_batch(users)
-        User.bulk_save(users, fields=("contributions_count",))

1-58: Add proper command execution status and verbosity control.

Django management commands typically return status codes and support verbosity levels. Consider enhancing the command to respect these conventions.

    def add_arguments(self, parser):
        """Add command-line arguments to the parser."""
        parser.add_argument("--offset", default=0, required=False, type=int)
+        # Django already adds --verbosity, we just need to use it

    def handle(self, *args, **options):
        """Handle the command execution."""
        offset = options["offset"]
+        verbosity = options['verbosity']
        # ... existing code ...

        for idx, user in enumerate(active_users[offset:]):
            prefix = f"{idx + offset + 1} of {active_users_count - offset}"
-            print(f"{prefix:<10} {user.title}")
+            if verbosity >= 1:
+                self.stdout.write(f"{prefix:<10} {user.title}")

        # ... existing code ...
+        if verbosity >= 1:
+            self.stdout.write(self.style.SUCCESS(
+                f"Successfully updated contributions count for {active_users_count - offset} users"
+            ))
+        return 0  # Success status code
📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 8ef9c6e and 66c1723.

📒 Files selected for processing (10)
  • backend/Makefile (2 hunks)
  • backend/apps/github/graphql/nodes/user.py (1 hunks)
  • backend/apps/github/management/commands/github_update_users.py (1 hunks)
  • backend/apps/github/migrations/0023_alter_user_contributions_count.py (1 hunks)
  • backend/apps/github/models/user.py (3 hunks)
  • backend/tests/apps/github/graphql/nodes/user_test.py (1 hunks)
  • frontend/__tests__/unit/pages/UserDetails.test.tsx (4 hunks)
  • frontend/src/app/members/[memberKey]/page.tsx (3 hunks)
  • frontend/src/server/queries/userQueries.ts (1 hunks)
  • frontend/src/types/user.ts (3 hunks)
✅ Files skipped from review due to trivial changes (1)
  • backend/apps/github/migrations/0023_alter_user_contributions_count.py
🚧 Files skipped from review as they are similar to previous changes (7)
  • backend/apps/github/graphql/nodes/user.py
  • frontend/src/server/queries/userQueries.ts
  • backend/tests/apps/github/graphql/nodes/user_test.py
  • frontend/src/app/members/[memberKey]/page.tsx
  • backend/apps/github/models/user.py
  • frontend/tests/unit/pages/UserDetails.test.tsx
  • frontend/src/types/user.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
backend/apps/github/management/commands/github_update_users.py (2)
backend/apps/github/models/repository_contributor.py (1)
  • RepositoryContributor (12-104)
backend/apps/github/models/user.py (2)
  • User (12-122)
  • bulk_save (82-84)
⏰ Context from checks skipped due to timeout of 90000ms (4)
  • GitHub Check: Run frontend unit tests
  • GitHub Check: Run frontend e2e tests
  • GitHub Check: Run backend tests
  • GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (4)
backend/Makefile (2)

48-50: Well-implemented Makefile target for the new command.

The new github-update-users target correctly follows the established pattern in the Makefile:

  • Uses proper echo message for user feedback
  • Leverages the existing exec-backend-command mechanism
  • Command invocation matches the Django management command name

156-165: Good integration into the data update workflow.

Adding github-update-users to the update-data target prerequisites ensures that user contribution data is refreshed as part of the standard data update process. The placement is logical in the sequence of operations.

backend/apps/github/management/commands/github_update_users.py (2)

1-12: Good structure and imports for the Django management command.

The file has appropriate imports for Django command handling, logging, and database operations. The relevant models (RepositoryContributor and User) are properly imported.


14-25: Well-implemented command class with good documentation.

The command definition follows Django's best practices with:

  • Clear help text
  • Well-documented argument parser
  • Useful offset parameter that allows for resuming interrupted operations

Copy link

@arkid15r arkid15r enabled auto-merge April 20, 2025 04:01
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.

LGTM

@arkid15r arkid15r added this pull request to the merge queue Apr 20, 2025
Merged via the queue into OWASP:main with commit 2535211 Apr 20, 2025
22 checks passed
@ahmedxgouda ahmedxgouda deleted the feature/extend-user-model branch April 20, 2025 04:26
@ahmedxgouda
Copy link
Collaborator Author

LGTM

It seems that I understood during the data sync as during updating the data. A new management command made things much simpler and easier. Thank you for reviewing this.

shdwcodr pushed a commit to shdwcodr/Nest that referenced this pull request Jun 5, 2025
* Add contributions_count field to User model and update related logic.

* Refactor User model to calculate contributions_count during data update and improve migration formatting.

* Add contributionsCount to user queries and update UserDetails page.

* Update backend tests.

* Update frontend tests.

* Resolve conflicts

* Add contributions_count field to user model in migrations

* Refactor migration file for user contributions count to use consistent quotation marks

* Merge migrations

* Refactor migration dependencies formatting

* Update code

* Optimize code

---------

Co-authored-by: Arkadii Yakovets <[email protected]>
@coderabbitai coderabbitai bot mentioned this pull request Aug 13, 2025
2 tasks
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.

Extend User Model with contributions_count Field
2 participants