Merge pull request #409 from github-community-projects/fix/contributors-missing-rate-limit

fix: use commits-first approach to avoid rate limiting and missing contributors

Author: zkoppert

contributors.py

@@ -155,41 +155,53 @@ def get_contributors(repo: object, start_date: str, end_date: str, ghe: str):
     Returns:
         contributors (list): A list of ContributorStats objects
     """
-    all_repo_contributors = repo.contributors()
     contributors = []
+    endpoint = ghe if ghe else "https://github.com"
     try:
-        for user in all_repo_contributors:
-            # Ignore contributors with [bot] in their name
-            if "[bot]" in user.login:
-                continue
-
-            # Check if user has commits in the date range
-            if start_date and end_date:
-                user_commits = repo.commits(
-                    author=user.login, since=start_date, until=end_date
+        if start_date and end_date:
+            # Fetch commits in the date range and extract unique authors.
+            # This is much more efficient than iterating all-time contributors
+            # and checking each one for commits, which causes rate limiting
+            # on large repositories.
+            contributor_data = {}
+            for commit in repo.commits(since=start_date, until=end_date):
+                if commit.author is None:
+                    continue
+                login = commit.author.login
+                if "[bot]" in login:
+                    continue
+                if login not in contributor_data:
+                    contributor_data[login] = {
+                        "avatar_url": commit.author.avatar_url,
+                        "contribution_count": 0,
+                    }
+                contributor_data[login]["contribution_count"] += 1
+
+            for username, data in contributor_data.items():
+                commit_url = f"{endpoint}/{repo.full_name}/commits?author={username}&since={start_date}&until={end_date}"
+                contributor = contributor_stats.ContributorStats(
+                    username,
+                    False,
+                    data["avatar_url"],
+                    data["contribution_count"],
+                    commit_url,
+                    "",
                 )
-
-                # If the user has no commits in the date range, skip them
-                try:
-                    next(user_commits)
-                except StopIteration:
+                contributors.append(contributor)
+        else:
+            for user in repo.contributors():
+                if "[bot]" in user.login:
                     continue
-
-            # Store the contributor information in a ContributorStats object
-            endpoint = ghe if ghe else "https://github.com"
-            if start_date and end_date:
-                commit_url = f"{endpoint}/{repo.full_name}/commits?author={user.login}&since={start_date}&until={end_date}"
-            else:
                 commit_url = f"{endpoint}/{repo.full_name}/commits?author={user.login}"
-            contributor = contributor_stats.ContributorStats(
-                user.login,
-                False,
-                user.avatar_url,
-                user.contributions_count,
-                commit_url,
-                "",
-            )
-            contributors.append(contributor)
+                contributor = contributor_stats.ContributorStats(
+                    user.login,
+                    False,
+                    user.avatar_url,
+                    user.contributions_count,
+                    commit_url,
+                    "",
+                )
+                contributors.append(contributor)
     except Exception as e:
         print(f"Error getting contributors for repository: {repo.full_name}")
         print(e)

markdown.py

@@ -230,7 +230,10 @@ def get_contributor_table(
     sponsor_info = _is_truthy(sponsor_info)
     show_avatar = _is_truthy(show_avatar)
     link_to_profile = _is_truthy(link_to_profile)
-    columns = ["Username", "All Time Contribution Count"]
+    if start_date and end_date:
+        columns = ["Username", "Contribution Count"]
+    else:
+        columns = ["Username", "All Time Contribution Count"]
     if show_avatar:
         columns.insert(0, "Avatar")
     if start_date and end_date:

test_contributors.py

@@ -2,7 +2,7 @@
 
 import runpy
 import unittest
-from unittest.mock import MagicMock, call, patch
+from unittest.mock import MagicMock, patch
 
 import contributors as contributors_module
 from contributor_stats import ContributorStats
@@ -19,24 +19,27 @@ def test_get_contributors(self, mock_contributor_stats):
         Test the get_contributors function.
         """
         mock_repo = MagicMock()
-        mock_user = MagicMock()
-        mock_user.login = "user"
-        mock_user.avatar_url = "https://avatars.githubusercontent.com/u/12345678?v=4"
-        mock_user.contributions_count = 100
-        mock_repo.contributors.return_value = [mock_user]
+        mock_commit = MagicMock()
+        mock_commit.author.login = "user"
+        mock_commit.author.avatar_url = (
+            "https://avatars.githubusercontent.com/u/12345678?v=4"
+        )
         mock_repo.full_name = "owner/repo"
-        mock_repo.commits.return_value = iter([object()])
+        mock_repo.commits.return_value = iter([mock_commit])
 
-        contributors_module.get_contributors(mock_repo, "2022-01-01", "2022-12-31", "")
+        result = contributors_module.get_contributors(
+            mock_repo, "2022-01-01", "2022-12-31", ""
+        )
 
+        self.assertEqual(len(result), 1)
         mock_repo.commits.assert_called_once_with(
-            author="user", since="2022-01-01", until="2022-12-31"
+            since="2022-01-01", until="2022-12-31"
         )
         mock_contributor_stats.assert_called_once_with(
             "user",
             False,
             "https://avatars.githubusercontent.com/u/12345678?v=4",
-            100,
+            1,
             "https://github.com/owner/repo/commits?author=user&since=2022-01-01&until=2022-12-31",
             "",
         )
@@ -124,41 +127,34 @@ def test_get_all_contributors_with_repository(self, mock_get_contributors):
         )
 
     @patch("contributors.contributor_stats.ContributorStats")
-    def test_get_contributors_skip_users_with_no_commits(self, mock_contributor_stats):
+    def test_get_contributors_with_single_commit(self, mock_contributor_stats):
         """
-        Test the get_contributors function skips users with no commits in the date range.
+        Test get_contributors returns a single contributor for one commit in the date range.
         """
         mock_repo = MagicMock()
-        mock_user = MagicMock()
-        mock_user.login = "user"
-        mock_user.avatar_url = "https://avatars.githubusercontent.com/u/12345678?v=4"
-        mock_user.contributions_count = 100
-        mock_user2 = MagicMock()
-        mock_user2.login = "user2"
-        mock_user2.avatar_url = "https://avatars.githubusercontent.com/u/12345679?v=4"
-        mock_user2.contributions_count = 102
+        mock_commit = MagicMock()
+        mock_commit.author.login = "user"
+        mock_commit.author.avatar_url = (
+            "https://avatars.githubusercontent.com/u/12345678?v=4"
+        )
 
-        mock_repo.contributors.return_value = [mock_user, mock_user2]
         mock_repo.full_name = "owner/repo"
-        mock_repo.commits.side_effect = [
-            iter([object()]),  # user has commits in range
-            iter([]),  # user2 has no commits in range and should be skipped
-        ]
+        mock_repo.commits.return_value = iter([mock_commit])
         ghe = ""
 
-        contributors_module.get_contributors(mock_repo, "2022-01-01", "2022-12-31", ghe)
+        result = contributors_module.get_contributors(
+            mock_repo, "2022-01-01", "2022-12-31", ghe
+        )
 
-        mock_repo.commits.assert_has_calls(
-            [
-                call(author="user", since="2022-01-01", until="2022-12-31"),
-                call(author="user2", since="2022-01-01", until="2022-12-31"),
-            ]
+        self.assertEqual(len(result), 1)
+        mock_repo.commits.assert_called_once_with(
+            since="2022-01-01", until="2022-12-31"
         )
         mock_contributor_stats.assert_called_once_with(
             "user",
             False,
             "https://avatars.githubusercontent.com/u/12345678?v=4",
-            100,
+            1,
             "https://github.com/owner/repo/commits?author=user&since=2022-01-01&until=2022-12-31",
             "",
         )
@@ -169,19 +165,22 @@ def test_get_contributors_skip_bot(self, mock_contributor_stats):
         Test if the get_contributors function skips the bot user.
         """
         mock_repo = MagicMock()
-        mock_user = MagicMock()
-        mock_user.login = "[bot]"
-        mock_user.avatar_url = "https://avatars.githubusercontent.com/u/12345678?v=4"
-        mock_user.contributions_count = 100
+        mock_commit = MagicMock()
+        mock_commit.author.login = "[bot]"
+        mock_commit.author.avatar_url = (
+            "https://avatars.githubusercontent.com/u/12345678?v=4"
+        )
 
-        mock_repo.contributors.return_value = [mock_user]
         mock_repo.full_name = "owner/repo"
+        mock_repo.commits.return_value = iter([mock_commit])
         ghe = ""
 
-        contributors_module.get_contributors(mock_repo, "2022-01-01", "2022-12-31", ghe)
+        result = contributors_module.get_contributors(
+            mock_repo, "2022-01-01", "2022-12-31", ghe
+        )
 
+        self.assertEqual(result, [])
         # Ensure that the bot user is skipped and ContributorStats is never instantiated
-        mock_repo.commits.assert_not_called()
         mock_contributor_stats.assert_not_called()
 
     @patch("contributors.contributor_stats.ContributorStats")
@@ -212,13 +211,8 @@ def test_get_contributors_no_commit_end_date(self, mock_contributor_stats):
         )
 
     def test_get_contributors_skips_when_no_commits_in_range(self):
-        """Test get_contributors skips users with no commits in the date range."""
+        """Test get_contributors returns empty list when no commits in the date range."""
         mock_repo = MagicMock()
-        mock_user = MagicMock()
-        mock_user.login = "user"
-        mock_user.avatar_url = "https://avatars.githubusercontent.com/u/12345678?v=4"
-        mock_user.contributions_count = 100
-        mock_repo.contributors.return_value = [mock_user]
         mock_repo.full_name = "owner/repo"
         mock_repo.commits.return_value = iter([])
 
@@ -228,6 +222,39 @@ def test_get_contributors_skips_when_no_commits_in_range(self):
 
         self.assertEqual(result, [])
 
+    def test_get_contributors_skips_none_author(self):
+        """Test get_contributors skips commits with no linked GitHub author."""
+        mock_repo = MagicMock()
+        mock_repo.full_name = "owner/repo"
+        mock_commit = MagicMock()
+        mock_commit.author = None
+        mock_repo.commits.return_value = iter([mock_commit])
+
+        result = contributors_module.get_contributors(
+            mock_repo, "2022-01-01", "2022-12-31", ""
+        )
+
+        self.assertEqual(result, [])
+
+    def test_get_contributors_aggregates_multiple_commits(self):
+        """Test get_contributors counts multiple commits per author correctly."""
+        mock_repo = MagicMock()
+        mock_repo.full_name = "owner/repo"
+        mock_commit1 = MagicMock()
+        mock_commit1.author.login = "user"
+        mock_commit1.author.avatar_url = "https://avatars.githubusercontent.com/u/1"
+        mock_commit2 = MagicMock()
+        mock_commit2.author.login = "user"
+        mock_commit2.author.avatar_url = "https://avatars.githubusercontent.com/u/1"
+        mock_repo.commits.return_value = iter([mock_commit1, mock_commit2])
+
+        result = contributors_module.get_contributors(
+            mock_repo, "2022-01-01", "2022-12-31", ""
+        )
+
+        self.assertEqual(len(result), 1)
+        self.assertEqual(result[0].contribution_count, 2)
+
     def test_get_contributors_handles_exception(self):
         """Test get_contributors returns None when an exception is raised."""
 
@@ -239,7 +266,7 @@ def __iter__(self):
 
         mock_repo = MagicMock()
         mock_repo.full_name = "owner/repo"
-        mock_repo.contributors.return_value = BoomIterable()
+        mock_repo.commits.return_value = BoomIterable()
 
         with patch("builtins.print") as mock_print:
             result = contributors_module.get_contributors(

test_markdown.py

@@ -67,7 +67,7 @@ def test_write_to_markdown(
             "| Total Contributors | Total Contributions | % New Contributors |\n"
             "| --- | --- | --- |\n"
             "| 2 | 300 | 50.0% |\n\n"
-            "| Username | All Time Contribution Count | New Contributor | "
+            "| Username | Contribution Count | New Contributor | "
             "Commits between 2023-01-01 and 2023-01-02 |\n"
             "| --- | --- | --- | --- |\n"
             "| @user1 | 100 | False | commit url |\n"
@@ -133,7 +133,7 @@ def test_write_to_markdown_with_sponsors(
             "| Total Contributors | Total Contributions | % New Contributors |\n"
             "| --- | --- | --- |\n"
             "| 2 | 300 | 50.0% |\n\n"
-            "| Username | All Time Contribution Count | New Contributor | "
+            "| Username | Contribution Count | New Contributor | "
             "Sponsor URL | Commits between 2023-01-01 and 2023-01-02 |\n"
             "| --- | --- | --- | --- | --- |\n"
             "| @user1 | 100 | False | [Sponsor Link](sponsor_url_1) | commit url |\n"
@@ -200,7 +200,7 @@ def test_write_to_markdown_with_avatars(
             "| Total Contributors | Total Contributions | % New Contributors |\n"
             "| --- | --- | --- |\n"
             "| 2 | 300 | 50.0% |\n\n"
-            "| Avatar | Username | All Time Contribution Count | New Contributor | "
+            "| Avatar | Username | Contribution Count | New Contributor | "
             "Commits between 2023-01-01 and 2023-01-02 |\n"
             "| --- | --- | --- | --- | --- |\n"
             '| <img src="https://avatars.example.com/user1.png" width="32" height="32" /> | '
@@ -268,7 +268,7 @@ def test_write_to_markdown_without_link_to_profile(
             "| Total Contributors | Total Contributions | % New Contributors |\n"
             "| --- | --- | --- |\n"
             "| 2 | 300 | 50.0% |\n\n"
-            "| Username | All Time Contribution Count | New Contributor | "
+            "| Username | Contribution Count | New Contributor | "
             "Commits between 2023-01-01 and 2023-01-02 |\n"
             "| --- | --- | --- | --- |\n"
             "| user1 | 100 | False | commit url |\n"
@@ -394,7 +394,7 @@ def test_write_to_markdown_with_organization(
             "| Total Contributors | Total Contributions | % New Contributors |\n"
             "| --- | --- | --- |\n"
             "| 2 | 300 | 50.0% |\n\n"
-            "| Username | All Time Contribution Count | New Contributor | "
+            "| Username | Contribution Count | New Contributor | "
             "Commits between 2023-01-01 and 2023-01-02 |\n"
             "| --- | --- | --- | --- |\n"
             "| @user1 | 100 | False | "
@@ -442,7 +442,7 @@ def test_write_to_markdown_empty_collaborators(
             "| Total Contributors | Total Contributions | % New Contributors |\n"
             "| --- | --- | --- |\n"
             "| 0 | 0 | 0% |\n\n"
-            "| Username | All Time Contribution Count | New Contributor | "
+            "| Username | Contribution Count | New Contributor | "
             "Commits between 2023-01-01 and 2023-01-02 |\n"
             "| --- | --- | --- | --- |\n"
             "\n _this file was generated by the "
@@ -555,7 +555,7 @@ def test_write_to_markdown_with_ghe(
             "| Total Contributors | Total Contributions | % New Contributors |\n"
             "| --- | --- | --- |\n"
             "| 1 | 100 | 0.0% |\n\n"
-            "| Username | All Time Contribution Count | New Contributor | "
+            "| Username | Contribution Count | New Contributor | "
             "Commits between 2023-01-01 and 2023-01-02 |\n"
             "| --- | --- | --- | --- |\n"
             "| @user1 | 100 | False | "