Merge pull request github#261 from github/multi-repo-search-queries

zkoppert · web-flow · commit d40d4835cb2b · 2024-04-30T10:25:36.000-07:00
fix: handle multiple repos in search query
diff --git a/issue_metrics.py b/issue_metrics.py
@@ -8,7 +8,7 @@
 Functions:
     get_env_vars() -> EnvVars: Get the environment variables for use
         in the script.
-    search_issues(search_query: str, github_connection: github3.GitHub)
+    search_issues(search_query: str, github_connection: github3.GitHub, owners_and_repositories: List[dict])
         -> github3.structs.SearchIterator:
         Searches for issues in a GitHub repository that match the given search query.
     get_per_issue_metrics(issues: Union[List[dict], List[github3.issues.Issue]],
@@ -43,7 +43,9 @@
 
 
 def search_issues(
-    search_query: str, github_connection: github3.GitHub, owner: str, repository: str
+    search_query: str,
+    github_connection: github3.GitHub,
+    owners_and_repositories: List[dict],
 ) -> List[github3.search.IssueSearchResult]:  # type: ignore
     """
     Searches for issues/prs/discussions in a GitHub repository that match
@@ -52,8 +54,8 @@ def search_issues(
     Args:
         search_query (str): The search query to use for finding issues/prs/discussions.
         github_connection (github3.GitHub): A connection to the GitHub API.
-        owner (str): The owner of the repository to search in.
-        repository (str): The repository to search in.
+        owners_and_repositories (List[dict]): A list of dictionaries containing
+            the owner and repository names.
 
     Returns:
         List[github3.search.IssueSearchResult]: A list of issues that match the search query.
@@ -63,18 +65,22 @@ def search_issues(
 
     # Print the issue titles
     issues = []
+    repos_and_owners_string = ""
+    for item in owners_and_repositories:
+        repos_and_owners_string += f"{item['owner']}/{item['repository']} "
+
     try:
         for issue in issues_iterator:
             print(issue.title)  # type: ignore
             issues.append(issue)
     except github3.exceptions.ForbiddenError:
         print(
-            f"You do not have permission to view this repository '{repository}'; Check your API Token."
+            f"You do not have permission to view a repository from: '{repos_and_owners_string}'; Check your API Token."
         )
         sys.exit(1)
     except github3.exceptions.NotFoundError:
         print(
-            f"The repository could not be found; Check the repository owner and name: '{owner}/{repository}"
+            f"The repository could not be found; Check the repository owner and names: '{repos_and_owners_string}"
         )
         sys.exit(1)
     except github3.exceptions.ConnectionError:
@@ -212,28 +218,35 @@ def get_per_issue_metrics(
     return issues_with_metrics, num_issues_open, num_issues_closed
 
 
-def get_owner_and_repository(
+def get_owners_and_repositories(
     search_query: str,
-) -> dict:
-    """Get the owner and repository from the search query.
+) -> List[dict]:
+    """Get the owners and repositories from the search query.
 
     Args:
         search_query (str): The search query used to search for issues.
 
     Returns:
-        dict: A dictionary of owner and repository.
+        List[dict]: A list of dictionaries of owners and repositories.
 
     """
     search_query_split = search_query.split(" ")
-    result = {}
+    results_list = []
     for item in search_query_split:
+        result = {}
         if "repo:" in item and "/" in item:
             result["owner"] = item.split(":")[1].split("/")[0]
             result["repository"] = item.split(":")[1].split("/")[1]
         if "org:" in item or "owner:" in item or "user:" in item:
             result["owner"] = item.split(":")[1]
+        if "user:" in item:
+            result["owner"] = item.split(":")[1]
+        if "owner:" in item:
+            result["owner"] = item.split(":")[1]
+        if result:
+            results_list.append(result)
 
-    return result
+    return results_list
 
 
 def main():
@@ -279,17 +292,17 @@ def main():
     max_comments_eval = int(env_vars.max_comments_eval)
     heavily_involved_cutoff = int(env_vars.heavily_involved_cutoff)
 
-    # Get the owner and repository from the search query
-    owner_and_repository = get_owner_and_repository(search_query)
-    owner = owner_and_repository.get("owner")
-    repository = owner_and_repository.get("repository")
+    # Get the owners and repositories from the search query
+    owners_and_repositories = get_owners_and_repositories(search_query)
 
-    if owner is None:
-        raise ValueError(
-            "The search query must include a repository owner and name \
-            (ie. repo:owner/repo), an organization (ie. org:organization), \
-            a user (ie. user:login) or an owner (ie. owner:user-or-organization)"
-        )
+    # Every search query must include a repository owner for each repository, organization, or user
+    for item in owners_and_repositories:
+        if item["owner"] is None:
+            raise ValueError(
+                "The search query must include a repository owner and name \
+                (ie. repo:owner/repo), an organization (ie. org:organization), \
+                a user (ie. user:login) or an owner (ie. owner:user-or-organization)"
+            )
 
     # Determine if there are label to measure
     labels = env_vars.labels_to_measure
@@ -307,7 +320,7 @@ def main():
             write_to_markdown(None, None, None, None, None, None, None, None)
             return
     else:
-        issues = search_issues(search_query, github_connection, owner, repository)
+        issues = search_issues(search_query, github_connection, owners_and_repositories)
         if len(issues) <= 0:
             print("No issues found")
             write_to_markdown(None, None, None, None, None, None, None, None)
diff --git a/test_issue_metrics.py b/test_issue_metrics.py
@@ -17,11 +17,10 @@
 from datetime import datetime, timedelta
 from unittest.mock import MagicMock, patch
 
-import issue_metrics
 from issue_metrics import (
     IssueWithMetrics,
     get_env_vars,
-    get_owner_and_repository,
+    get_owners_and_repositories,
     get_per_issue_metrics,
     measure_time_to_close,
     measure_time_to_first_response,
@@ -52,43 +51,62 @@ def test_search_issues(self):
         mock_connection.search_issues.return_value = mock_issues
 
         # Call search_issues and check that it returns the correct issues
-        issues = search_issues(
-            "is:open", mock_connection, "fakeowner", "fakerepository"
-        )
+        repo_with_owner = {"owner": "owner1", "repository": "repo1"}
+        owners_and_repositories = [repo_with_owner]
+        issues = search_issues("is:open", mock_connection, owners_and_repositories)
         self.assertEqual(issues, mock_issues)
 
 
 class TestGetOwnerAndRepository(unittest.TestCase):
-    """Unit tests for the get_owner_and_repository function.
+    """Unit tests for the get_owners_and_repositories function.
 
-    This class contains unit tests for the get_owner_and_repository function in the
+    This class contains unit tests for the get_owners_and_repositories function in the
     issue_metrics module. The tests use the unittest module and the unittest.mock
     module to mock the GitHub API and test the function in isolation.
 
     Methods:
-        test_get_owner_with_owner_and_repo_in_query: Test get both owner and repo.
-        test_get_owner_and_repository_with_repo_in_query: Test get just owner.
-        test_get_owner_and_repository_without_either_in_query: Test get neither.
-
+        test_get_owners_with_owner_and_repo_in_query: Test get both owner and repo.
+        test_get_owners_and_repositories_with_repo_in_query: Test get just owner.
+        test_get_owners_and_repositories_without_either_in_query: Test get neither.
+        test_get_owners_and_repositories_with_multiple_entries: Test get multiple entries.
     """
 
-    def test_get_owner_with_owner_and_repo_in_query(self):
+    def test_get_owners_with_owner_and_repo_in_query(self):
         """Test get both owner and repo."""
-        result = get_owner_and_repository("repo:owner1/repo1")
-        self.assertEqual(result.get("owner"), "owner1")
-        self.assertEqual(result.get("repository"), "repo1")
+        result = get_owners_and_repositories("repo:owner1/repo1")
+        self.assertEqual(result[0].get("owner"), "owner1")
+        self.assertEqual(result[0].get("repository"), "repo1")
 
-    def test_get_owner_and_repository_with_repo_in_query(self):
+    def test_get_owner_and_repositories_with_repo_in_query(self):
         """Test get just owner."""
-        result = get_owner_and_repository("org:owner1")
-        self.assertEqual(result.get("owner"), "owner1")
-        self.assertIsNone(result.get("repository"))
+        result = get_owners_and_repositories("org:owner1")
+        self.assertEqual(result[0].get("owner"), "owner1")
+        self.assertIsNone(result[0].get("repository"))
 
-    def test_get_owner_and_repository_without_either_in_query(self):
+    def test_get_owners_and_repositories_without_either_in_query(self):
         """Test get neither."""
-        result = get_owner_and_repository("is:blah")
-        self.assertIsNone(result.get("owner"))
-        self.assertIsNone(result.get("repository"))
+        result = get_owners_and_repositories("is:blah")
+        self.assertEqual(result, [])
+
+    def test_get_owners_and_repositories_with_multiple_entries(self):
+        """Test get multiple entries."""
+        result = get_owners_and_repositories("repo:owner1/repo1 org:owner2")
+        self.assertEqual(result[0].get("owner"), "owner1")
+        self.assertEqual(result[0].get("repository"), "repo1")
+        self.assertEqual(result[1].get("owner"), "owner2")
+        self.assertIsNone(result[1].get("repository"))
+
+    def test_get_owners_and_repositories_with_org(self):
+        """Test get org as owner."""
+        result = get_owners_and_repositories("org:owner1")
+        self.assertEqual(result[0].get("owner"), "owner1")
+        self.assertIsNone(result[0].get("repository"))
+
+    def test_get_owners_and_repositories_with_user(self):
+        """Test get user as owner."""
+        result = get_owners_and_repositories("user:owner1")
+        self.assertEqual(result[0].get("owner"), "owner1")
+        self.assertIsNone(result[0].get("repository"))
 
 
 class TestGetEnvVars(unittest.TestCase):
@@ -120,115 +138,6 @@ def test_get_env_vars_missing_query(self):
             get_env_vars(test=True)
 
 
-class TestMain(unittest.TestCase):
-    """Unit tests for the main function.
-
-    This class contains unit tests for the main function in the issue_metrics
-    module. The tests use the unittest module and the unittest.mock module to
-    mock the GitHub API and test the function in isolation.
-
-    Methods:
-        test_main: Test that main runs without errors.
-        test_main_no_issues_found: Test that main handles when no issues are found
-
-    """
-
-    @patch("issue_metrics.auth_to_github")
-    @patch("issue_metrics.search_issues")
-    @patch("issue_metrics.measure_time_to_first_response")
-    @patch("issue_metrics.get_stats_time_to_first_response")
-    @patch.dict(
-        os.environ,
-        {
-            "SEARCH_QUERY": "is:open repo:user/repo",
-            "GH_TOKEN": "test_token",
-        },
-    )
-    def test_main(
-        self,
-        mock_get_stats_time_to_first_response,
-        mock_measure_time_to_first_response,
-        mock_search_issues,
-        mock_auth_to_github,
-    ):
-        """Test that main runs without errors."""
-        # Set up the mock GitHub connection object
-        mock_connection = MagicMock()
-        mock_auth_to_github.return_value = mock_connection
-
-        # Set up the mock search_issues function
-        mock_issues = MagicMock(
-            items=[
-                MagicMock(title="Issue 1"),
-                MagicMock(title="Issue 2"),
-            ]
-        )
-
-        mock_search_issues.return_value = mock_issues
-
-        # Set up the mock measure_time_to_first_response function
-        mock_issues_with_ttfr = [
-            (
-                "Issue 1",
-                "https://github.com/user/repo/issues/1",
-                "alice",
-                timedelta(days=1, hours=2, minutes=30),
-            ),
-            (
-                "Issue 2",
-                "https://github.com/user/repo/issues/2",
-                "bob",
-                timedelta(days=3, hours=4, minutes=30),
-            ),
-        ]
-        mock_measure_time_to_first_response.return_value = mock_issues_with_ttfr
-
-        # Set up the mock get_stats_time_to_first_response function
-        mock_stats_time_to_first_response = 15
-        mock_get_stats_time_to_first_response.return_value = (
-            mock_stats_time_to_first_response
-        )
-
-        # Call main and check that it runs without errors
-        issue_metrics.main()
-
-        # Remove the markdown file created by main
-        os.remove("issue_metrics.md")
-
-    @patch("issue_metrics.auth_to_github")
-    @patch("issue_metrics.search_issues")
-    @patch("issue_metrics.write_to_markdown")
-    @patch.dict(
-        os.environ,
-        {
-            "SEARCH_QUERY": "is:open repo:org/repo",
-            "GH_TOKEN": "test_token",
-        },
-    )
-    def test_main_no_issues_found(
-        self,
-        mock_write_to_markdown,
-        mock_search_issues,
-        mock_auth_to_github,
-    ):
-        """Test that main writes 'No issues found' to the
-        console and calls write_to_markdown with None."""
-
-        # Set up the mock GitHub connection object
-        mock_connection = MagicMock()
-        mock_auth_to_github.return_value = mock_connection
-
-        # Set up the mock search_issues function to return an empty list of issues
-        mock_issues = MagicMock(items=[])
-        mock_search_issues.return_value = mock_issues
-
-        # Call main and check that it writes 'No issues found'
-        issue_metrics.main()
-        mock_write_to_markdown.assert_called_once_with(
-            None, None, None, None, None, None, None, None
-        )
-
-
 class TestGetPerIssueMetrics(unittest.TestCase):
     """Test suite for the get_per_issue_metrics function."""
 
