diff --git a/sync2jira/downstream_issue.py b/sync2jira/downstream_issue.py index 173b1b68..036cabe5 100644 --- a/sync2jira/downstream_issue.py +++ b/sync2jira/downstream_issue.py @@ -1295,7 +1295,8 @@ def _update_github_project_fields( for name, values in github_project_fields.items(): if name not in dir(issue): log.error( - f"Configuration error: github_project_field key, {name:r}, is not in issue object." + "Configuration error: github_project_field key, %r, is not in issue object.", + name, ) continue diff --git a/sync2jira/downstream_pr.py b/sync2jira/downstream_pr.py index 76ff13f5..bb328e7e 100644 --- a/sync2jira/downstream_pr.py +++ b/sync2jira/downstream_pr.py @@ -357,7 +357,7 @@ def _create_jira_issue_from_pr(client, pr, config): assignee=pr.assignee or [], status=pr.status, id_=pr.id, - storypoints=None, + storypoints=pr.storypoints, upstream_id=pr.id, issue_type=None, downstream=pr.downstream, # Use PR's downstream config diff --git a/sync2jira/intermediary.py b/sync2jira/intermediary.py index efd37c70..95feb1cc 100644 --- a/sync2jira/intermediary.py +++ b/sync2jira/intermediary.py @@ -150,6 +150,7 @@ def __init__( tags, fixVersion, priority, + storypoints, content, reporter, assignee, @@ -169,6 +170,7 @@ def __init__( self.tags = tags self.fixVersion = fixVersion self.priority = priority + self.storypoints = storypoints self.base_branch = base_branch # JIRA treats utf-8 characters in ways we don't totally understand, so scrub content down to @@ -245,7 +247,8 @@ def from_github(cls, upstream, pr, suffix, config, action=None): comments=comments, tags=pr.get("labels", []), fixVersion=[pr.get("milestone")], - priority=None, + priority=pr.get("priority"), + storypoints=pr.get("storypoints"), content=pr.get("body"), reporter=pr["user"]["fullname"], assignee=pr["assignee"], diff --git a/sync2jira/upstream_issue.py b/sync2jira/upstream_issue.py index 1e66a693..76d9d51c 100644 --- a/sync2jira/upstream_issue.py +++ b/sync2jira/upstream_issue.py @@ -109,6 +109,25 @@ } """ +ghquery_pr = ghquery.replace( + "issue(number: $issuenumber)", "pullRequest(number: $issuenumber)" +) + + +def get_github_client(config): + """ + Helper function returning headers and github_client built from config. + + :param dict config: Config + :returns: (headers, github_client) + :rtype: tuple + """ + + token = config["sync2jira"].get("github_token") + headers = {"Authorization": "token " + token} if token else {} + github_client = Github(token, retry=5) + return headers, github_client + def passes_github_filters(item, config, upstream, item_type="issue"): """ @@ -194,9 +213,7 @@ def handle_github_message(body, config, is_pr=False): ) return None - token = config["sync2jira"].get("github_token") - headers = {"Authorization": "token " + token} if token else {} - github_client = Github(token, retry=5) + headers, github_client = get_github_client(config) reformat_github_issue(issue, upstream, github_client) add_project_values(issue, upstream, headers, config) return i.Issue.from_github(upstream, issue, config) @@ -211,9 +228,7 @@ def github_issues(upstream, config): :returns: a generator for GitHub Issue objects :rtype: Generator[sync2jira.intermediary.Issue] """ - token = config["sync2jira"].get("github_token") - headers = {"Authorization": "token " + token} if token else {} - github_client = Github(token, retry=5) + headers, github_client = get_github_client(config) for issue in generate_github_items("issues", upstream, config): if "pull_request" in issue or "/pull/" in issue.get("html_url", ""): # We don't want to copy these around @@ -230,18 +245,19 @@ def github_issues(upstream, config): yield i.Issue.from_github(upstream, issue, config) -def add_project_values(issue, upstream, headers, config): - """Add values to an issue from its corresponding card in a GitHub Project +def add_project_values(issue, upstream, headers, config, updates_key="issue_updates"): + """Add values to an issue/PR from its corresponding card in a GitHub Project - :param dict issue: Issue + :param dict issue: Issue or PR dict :param str upstream: Upstream repo name :param dict headers: HTTP Request headers, including access token, if any :param dict config: Config + :param str updates_key: Config key for the updates list """ upstream_config = config["sync2jira"]["map"]["github"][upstream] - issue_updates = upstream_config.get("issue_updates", []) + updates = upstream_config.get(updates_key, []) github_project_fields = upstream_config.get("github_project_fields") - if not github_project_fields or "github_project_fields" not in issue_updates: + if not github_project_fields or "github_project_fields" not in updates: log.debug( "github_project_fields is None or empty, skipping project field updates" ) @@ -252,12 +268,14 @@ def add_project_values(issue, upstream, headers, config): issuenumber = issue["number"] orgname, reponame = upstream.rsplit("/", 1) variables = {"orgname": orgname, "reponame": reponame, "issuenumber": issuenumber} + query = ghquery_pr if updates_key == "pr_updates" else ghquery response = requests.post( - graphqlurl, headers=headers, json={"query": ghquery, "variables": variables} + graphqlurl, headers=headers, json={"query": query, "variables": variables} ) if response.status_code != 200: log.info( - "HTTP error while fetching issue %s/%s#%s: %s", + "HTTP error while fetching %s %s/%s#%s: %s", + "PR" if updates_key == "pr_updates" else "issue", orgname, reponame, issuenumber, @@ -265,10 +283,12 @@ def add_project_values(issue, upstream, headers, config): ) return data = response.json() - gh_issue = data.get("data", {}).get("repository", {}).get("issue") - if not gh_issue: + repo_data = data.get("data", {}).get("repository", {}) + gh_item = repo_data.get("pullRequest" if updates_key == "pr_updates" else "issue") + if not gh_item: log.info( - "GitHub error while fetching issue %s/%s#%s: %s", + "GitHub error while fetching %s %s/%s#%s: %s", + "PR" if updates_key == "pr_updates" else "issue", orgname, reponame, issuenumber, @@ -276,7 +296,7 @@ def add_project_values(issue, upstream, headers, config): ) return project_node = _get_current_project_node( - upstream, project_number, issuenumber, gh_issue + upstream, project_number, issuenumber, gh_item ) if not project_node: return diff --git a/sync2jira/upstream_pr.py b/sync2jira/upstream_pr.py index 1a5d47d5..e86465b8 100644 --- a/sync2jira/upstream_pr.py +++ b/sync2jira/upstream_pr.py @@ -19,7 +19,7 @@ import logging -from github import Github, UnknownObjectException +from github import UnknownObjectException import sync2jira.intermediary as i import sync2jira.upstream_issue as u_issue @@ -44,9 +44,9 @@ def handle_github_message(body, config, suffix): pr = body["pull_request"] if not u_issue.passes_github_filters(pr, config, upstream, item_type="PR"): return None - token = config["sync2jira"].get("github_token") - github_client = Github(token, retry=5) + headers, github_client = u_issue.get_github_client(config) reformat_github_pr(pr, upstream, github_client) + u_issue.add_project_values(pr, upstream, headers, config, "pr_updates") return i.PR.from_github(upstream, pr, suffix, config, body.get("action")) @@ -59,9 +59,10 @@ def github_prs(upstream, config): :returns: a generator for GitHub PR objects :rtype: Generator[sync2jira.intermediary.PR] """ - github_client = Github(config["sync2jira"]["github_token"]) + headers, github_client = u_issue.get_github_client(config) for pr in u_issue.generate_github_items("pulls", upstream, config): reformat_github_pr(pr, upstream, github_client) + u_issue.add_project_values(pr, upstream, headers, config, "pr_updates") yield i.PR.from_github(upstream, pr, "open", config) diff --git a/tests/test_upstream_issue.py b/tests/test_upstream_issue.py index abc58b62..0dda73f8 100644 --- a/tests/test_upstream_issue.py +++ b/tests/test_upstream_issue.py @@ -1009,6 +1009,175 @@ def test_add_project_values_storypoints(self, mock_requests_post): ) mock_requests_post.reset_mock() + @mock.patch(PATH + "requests.post") + def test_add_project_values_pr_early_exit(self, mock_requests_post): + """Test add_project_values early exit when using pr_updates.""" + upstream_config = { + "pr_updates": ["comments", "title"], + "github_project_number": 1, + } + self.mock_config["sync2jira"]["map"]["github"]["org/repo"] = upstream_config + + mock_issue = {"number": 1234, "storypoints": None, "priority": None} + + scenarios = ( + ("github_project_fields is None", None, ["github_project_fields"]), + ("github_project_fields is empty", {}, ["github_project_fields"]), + ( + "github_project_fields not in pr_updates", + {"storypoints": {"gh_field": "Estimate"}}, + [], + ), + ) + for description, gpf, extra_updates in scenarios: + with self.subTest(description=description): + upstream_config["github_project_fields"] = gpf + upstream_config["pr_updates"] = ["comments", "title"] + extra_updates + result = u.add_project_values( + issue=mock_issue, + upstream="org/repo", + headers={}, + config=self.mock_config, + updates_key="pr_updates", + ) + mock_requests_post.assert_not_called() + self.assertIsNone(result) + mock_requests_post.reset_mock() + + @mock.patch(PATH + "requests.post") + def test_add_project_values_pr(self, mock_requests_post): + """Test add_project_values with pr_updates uses pullRequest query and response key. + + The storypoints/priority processing logic is shared with issues and + is thoroughly tested by test_add_project_values_storypoints. This + test focuses on the PR-specific behavior: reading from pr_updates, + sending the pullRequest GraphQL query, and parsing the pullRequest + response key. + """ + upstream_config = { + "pr_updates": ["github_project_fields"], + "github_project_number": 1, + } + self.mock_config["sync2jira"]["map"]["github"]["org/repo"] = upstream_config + + mock_issue = {"number": 1234, "storypoints": None, "priority": None} + + mock_requests_post.return_value.status_code = 200 + + scenarios = ( + ( + "Storypoints via Number field", + { + "priority": {"gh_field": "Priority"}, + "storypoints": {"gh_field": "Estimate"}, + }, + [ + {"fieldName": {"name": "Priority"}, "name": "High"}, + {"fieldName": {"name": "Estimate"}, "number": 5}, + ], + 5, + "High", + ), + ( + "Storypoints via Single Select", + { + "priority": {"gh_field": "Priority"}, + "storypoints": { + "gh_field": "Size", + "options": {"Small": 1, "Medium": 3, "Large": 8}, + }, + }, + [ + {"fieldName": {"name": "Size"}, "name": "Medium"}, + {"fieldName": {"name": "Priority"}, "name": "Critical"}, + ], + 3, + "Critical", + ), + ( + "Priority only, no storypoints config", + { + "priority": {"gh_field": "Priority"}, + }, + [ + {"fieldName": {"name": "Priority"}, "name": "Low"}, + ], + None, + "Low", + ), + ( + "Storypoints only, no priority config", + { + "storypoints": {"gh_field": "Estimate"}, + }, + [ + {"fieldName": {"name": "Estimate"}, "number": 8}, + ], + 8, + None, + ), + ) + + for description, gpf, field_nodes, expected_sp, expected_prio in scenarios: + with self.subTest(description=description): + upstream_config["github_project_fields"] = gpf + mock_issue["storypoints"] = None + mock_issue["priority"] = None + + mock_requests_post.return_value.json.return_value = { + "data": { + "repository": { + "pullRequest": { + "projectItems": { + "nodes": [ + { + "project": { + "title": "Project 1", + "number": 1, + }, + "fieldValues": {"nodes": field_nodes}, + } + ] + } + } + } + } + } + + u.add_project_values( + issue=mock_issue, + upstream="org/repo", + headers={}, + config=self.mock_config, + updates_key="pr_updates", + ) + + query_sent = mock_requests_post.call_args[1]["json"]["query"] + self.assertIn( + "pullRequest(number:", + query_sent, + "GraphQL query should use pullRequest, not issue", + ) + self.assertNotIn( + "issue(number:", + query_sent, + "GraphQL query should not contain issue(number:) for PRs", + ) + + self.assertEqual( + mock_issue["priority"], + expected_prio, + f"{description}: expected priority={expected_prio!r}, " + f"got {mock_issue['priority']!r}", + ) + self.assertEqual( + mock_issue.get("storypoints"), + expected_sp, + f"{description}: expected storypoints={expected_sp}, " + f"got {mock_issue.get('storypoints')}", + ) + mock_requests_post.reset_mock() + def test_passes_github_filters(self): """ Test passes_github_filters for labels, milestone, and other fields. diff --git a/tests/test_upstream_pr.py b/tests/test_upstream_pr.py index 20fefaad..2dc0ab37 100644 --- a/tests/test_upstream_pr.py +++ b/tests/test_upstream_pr.py @@ -6,6 +6,7 @@ import sync2jira.upstream_pr as u PATH = "sync2jira.upstream_pr." +ISSUE_PATH = "sync2jira.upstream_issue." class TestUpstreamPR(unittest.TestCase): @@ -82,7 +83,7 @@ def setUp(self): self.mock_github_client.get_repo.return_value = self.mock_github_repo self.mock_github_client.get_user.return_value = self.mock_github_person - @mock.patch(PATH + "Github") + @mock.patch(ISSUE_PATH + "Github") @mock.patch("sync2jira.intermediary.PR.from_github") def test_handle_github_message(self, mock_pr_from_github, mock_github): """ @@ -131,7 +132,7 @@ def test_handle_github_message(self, mock_pr_from_github, mock_github): self.mock_github_pr.get_issue_comments.assert_any_call() self.mock_github_client.get_user.assert_called_with("mock_login") - @mock.patch(PATH + "Github") + @mock.patch(ISSUE_PATH + "Github") @mock.patch("sync2jira.intermediary.Issue.from_github") def test_handle_github_message_not_in_mapped( self, mock_issue_from_github, mock_github @@ -155,7 +156,7 @@ def test_handle_github_message_not_in_mapped( self.assertEqual(None, response) @mock.patch("sync2jira.intermediary.PR.from_github") - @mock.patch(PATH + "Github") + @mock.patch(ISSUE_PATH + "Github") @mock.patch(PATH + "u_issue.get_all_github_data") def test_github_issues( self, mock_get_all_github_data, mock_github, mock_pr_from_github @@ -208,7 +209,52 @@ def test_github_issues( self.assertEqual(response[0], "Successful Call!") @mock.patch("sync2jira.intermediary.PR.from_github") - @mock.patch(PATH + "Github") + @mock.patch(PATH + "u_issue.add_project_values") + @mock.patch(PATH + "reformat_github_pr") + @mock.patch(PATH + "u_issue.generate_github_items") + @mock.patch(ISSUE_PATH + "Github") + def test_github_prs( + self, + mock_github, + mock_generate, + mock_reformat, + mock_add_project, + mock_pr_from_github, + ): + """Tests github_prs yields one PR object per item from generate_github_items. + + Three scenarios: zero, one, and multiple raw items. The subroutines + (reformat_github_pr, add_project_values, PR.from_github) are mocked + out because they are tested independently; all we care about here is + that github_prs produces the right number of results. + """ + mock_github.return_value = self.mock_github_client + + pr_a = {"number": 1} + pr_b = {"number": 2} + + for description, raw_items, expected_count in ( + ("zero items", [], 0), + ("one item", [pr_a], 1), + ("multiple items", [pr_a, pr_b], 2), + ): + with self.subTest(description=description): + mock_generate.return_value = iter(raw_items) + mock_pr_from_github.side_effect = [ + f"PR-{pr['number']}" for pr in raw_items + ] + + result = list( + u.github_prs(upstream="org/repo", config=self.mock_config) + ) + + expected = [f"PR-{pr['number']}" for pr in raw_items] + self.assertEqual(result, expected, description) + + mock_pr_from_github.reset_mock() + + @mock.patch("sync2jira.intermediary.PR.from_github") + @mock.patch(ISSUE_PATH + "Github") @mock.patch(PATH + "u_issue.get_all_github_data") def test_filter_multiple_labels( self, mock_get_all_github_data, mock_github, mock_issue_from_github @@ -274,7 +320,7 @@ def test_handle_github_message_filter_returns_false( mock_pr_from_github.assert_not_called() self.assertIsNone(response) - @mock.patch(PATH + "Github") + @mock.patch(ISSUE_PATH + "Github") @mock.patch("sync2jira.upstream_pr.u_issue.passes_github_filters") @mock.patch("sync2jira.intermediary.PR.from_github") def test_handle_github_message_filter_returns_true(