From fad2af0bf993f149a6828aab4c0c3971a614f7c1 Mon Sep 17 00:00:00 2001 From: Bala Sakabattula Date: Wed, 25 Mar 2026 00:12:15 +0530 Subject: [PATCH 1/3] pr-issue-transition-fix --- sync2jira/intermediary.py | 23 ++++++++++++++--------- sync2jira/upstream_pr.py | 4 ++-- tests/test_intermediary.py | 37 +++++++++++++++++++++++++++++++------ tests/test_upstream_pr.py | 2 ++ 4 files changed, 49 insertions(+), 17 deletions(-) diff --git a/sync2jira/intermediary.py b/sync2jira/intermediary.py index 0725098c..d8a88c44 100644 --- a/sync2jira/intermediary.py +++ b/sync2jira/intermediary.py @@ -201,7 +201,7 @@ def title(self): return "[%s] %s" % (self.upstream, self._title) @classmethod - def from_github(cls, upstream, pr, suffix, config): + def from_github(cls, upstream, pr, suffix, config, action=None): """Helper function to create an intermediary PR object.""" # Set our upstream source upstream_source = "github" @@ -215,15 +215,20 @@ def from_github(cls, upstream, pr, suffix, config): # Match to a JIRA match = matcher(pr.get("body"), comments) - # Figure out what state we're transitioning too - if "reopened" in suffix: - suffix = "reopened" - elif "closed" in suffix: - # Check if we're merging or closing - if pr["merged"]: - suffix = "merged" + _lifecycle = frozenset({"open", "merged", "closed", "reopened"}) + if action: + if action == "reopened": + suffix = "reopened" + elif action == "closed": + suffix = "merged" if pr.get("merged") else "closed" + elif action == "opened": + suffix = "open" else: - suffix = "closed" + suffix = "open" + elif suffix in _lifecycle: + pass + else: + suffix = "open" # Return our PR object return cls( diff --git a/sync2jira/upstream_pr.py b/sync2jira/upstream_pr.py index bd2131ff..1ca413f2 100644 --- a/sync2jira/upstream_pr.py +++ b/sync2jira/upstream_pr.py @@ -47,7 +47,7 @@ def handle_github_message(body, config, suffix): token = config["sync2jira"].get("github_token") github_client = Github(token, retry=5) reformat_github_pr(pr, upstream, github_client) - return i.PR.from_github(upstream, pr, suffix, config) + return i.PR.from_github(upstream, pr, suffix, config, action=body.get("action")) def github_prs(upstream, config): @@ -62,7 +62,7 @@ def github_prs(upstream, config): github_client = Github(config["sync2jira"]["github_token"]) for pr in u_issue.generate_github_items("pulls", upstream, config): reformat_github_pr(pr, upstream, github_client) - yield i.PR.from_github(upstream, pr, "open", config) + yield i.PR.from_github(upstream, pr, "open", config, action=None) def reformat_github_pr(pr, upstream, github_client): diff --git a/tests/test_intermediary.py b/tests/test_intermediary.py index 3c845132..81f33e35 100644 --- a/tests/test_intermediary.py +++ b/tests/test_intermediary.py @@ -227,18 +227,15 @@ def test_mapping_github(self): @mock.patch(PATH + "matcher") def test_from_github_pr_reopen(self, mock_matcher): - """ - This tests the message from GitHub for a PR - """ - # Set up return values + """PR reopen uses webhook action, not topic suffix.""" mock_matcher.return_value = "JIRA-1234" - # Call the function response = i.PR.from_github( upstream="github", pr=self.mock_github_pr, - suffix="reopened", + suffix="github.pull_request", config=self.mock_config, + action="reopened", ) # Assert that we made the calls correctly @@ -253,6 +250,34 @@ def test_from_github_pr_reopen(self, mock_matcher): self.mock_github_pr["body"], self.mock_github_pr["comments"] ) + @mock.patch(PATH + "matcher") + def test_from_github_pr_flat_topic_normalizes_suffix(self, mock_matcher): + """Flat topic: suffix from webhook action (+ merged when closed); else open.""" + mock_matcher.return_value = "JIRA-1" + flat = "github.pull_request" + _omit_action = object() + cases = ( + ("closed with merge", {"merged": True}, "closed", "merged"), + ("closed without merge", {"merged": False}, "closed", "closed"), + ("reopened", {}, "reopened", "reopened"), + ("opened", {}, "opened", "open"), + ("edited maps to open", {}, "edited", "open"), + ("missing action", {}, _omit_action, "open"), + ) + for name, pr_extra, action, expected in cases: + with self.subTest(name): + pr = {**self.mock_github_pr, **pr_extra} + base_kw = dict( + upstream="github", + pr=pr, + suffix=flat, + config=self.mock_config, + ) + if action is not _omit_action: + base_kw["action"] = action + response = i.PR.from_github(**base_kw) + self.assertEqual(response.suffix, expected) + def test_matcher(self): """This tests the matcher function""" # Found in content, no comments diff --git a/tests/test_upstream_pr.py b/tests/test_upstream_pr.py index 9ad92dec..c70197f7 100644 --- a/tests/test_upstream_pr.py +++ b/tests/test_upstream_pr.py @@ -122,6 +122,7 @@ def test_handle_github_message(self, mock_pr_from_github, mock_github): }, "mock_suffix", self.mock_config, + action=None, ) mock_github.assert_called_with("mock_token", retry=5) self.assertEqual("Successful Call!", response) @@ -200,6 +201,7 @@ def test_github_issues( }, "open", self.mock_config, + action=None, ) self.mock_github_client.get_repo.assert_called_with("org/repo") self.mock_github_repo.get_pull.assert_called_with(number="1234") From d519d19c26224523743b9f98a4b4b03fd0665570 Mon Sep 17 00:00:00 2001 From: Bala Sakabattula Date: Wed, 25 Mar 2026 23:52:48 +0530 Subject: [PATCH 2/3] fxes --- sync2jira/intermediary.py | 8 ++------ sync2jira/upstream_pr.py | 4 ++-- tests/test_intermediary.py | 5 ++--- tests/test_upstream_pr.py | 3 +-- 4 files changed, 7 insertions(+), 13 deletions(-) diff --git a/sync2jira/intermediary.py b/sync2jira/intermediary.py index d8a88c44..df91bcb6 100644 --- a/sync2jira/intermediary.py +++ b/sync2jira/intermediary.py @@ -215,19 +215,15 @@ def from_github(cls, upstream, pr, suffix, config, action=None): # Match to a JIRA match = matcher(pr.get("body"), comments) - _lifecycle = frozenset({"open", "merged", "closed", "reopened"}) + lifecycle = frozenset({"open", "merged", "closed", "reopened"}) if action: if action == "reopened": suffix = "reopened" elif action == "closed": suffix = "merged" if pr.get("merged") else "closed" - elif action == "opened": - suffix = "open" else: suffix = "open" - elif suffix in _lifecycle: - pass - else: + elif suffix not in lifecycle: suffix = "open" # Return our PR object diff --git a/sync2jira/upstream_pr.py b/sync2jira/upstream_pr.py index 1ca413f2..1a5d47d5 100644 --- a/sync2jira/upstream_pr.py +++ b/sync2jira/upstream_pr.py @@ -47,7 +47,7 @@ def handle_github_message(body, config, suffix): token = config["sync2jira"].get("github_token") github_client = Github(token, retry=5) reformat_github_pr(pr, upstream, github_client) - return i.PR.from_github(upstream, pr, suffix, config, action=body.get("action")) + return i.PR.from_github(upstream, pr, suffix, config, body.get("action")) def github_prs(upstream, config): @@ -62,7 +62,7 @@ def github_prs(upstream, config): github_client = Github(config["sync2jira"]["github_token"]) for pr in u_issue.generate_github_items("pulls", upstream, config): reformat_github_pr(pr, upstream, github_client) - yield i.PR.from_github(upstream, pr, "open", config, action=None) + yield i.PR.from_github(upstream, pr, "open", config) def reformat_github_pr(pr, upstream, github_client): diff --git a/tests/test_intermediary.py b/tests/test_intermediary.py index 81f33e35..d5bb3d47 100644 --- a/tests/test_intermediary.py +++ b/tests/test_intermediary.py @@ -255,14 +255,13 @@ def test_from_github_pr_flat_topic_normalizes_suffix(self, mock_matcher): """Flat topic: suffix from webhook action (+ merged when closed); else open.""" mock_matcher.return_value = "JIRA-1" flat = "github.pull_request" - _omit_action = object() cases = ( ("closed with merge", {"merged": True}, "closed", "merged"), ("closed without merge", {"merged": False}, "closed", "closed"), ("reopened", {}, "reopened", "reopened"), ("opened", {}, "opened", "open"), ("edited maps to open", {}, "edited", "open"), - ("missing action", {}, _omit_action, "open"), + ("missing action", {}, None, "open"), ) for name, pr_extra, action, expected in cases: with self.subTest(name): @@ -273,7 +272,7 @@ def test_from_github_pr_flat_topic_normalizes_suffix(self, mock_matcher): suffix=flat, config=self.mock_config, ) - if action is not _omit_action: + if action is not None: base_kw["action"] = action response = i.PR.from_github(**base_kw) self.assertEqual(response.suffix, expected) diff --git a/tests/test_upstream_pr.py b/tests/test_upstream_pr.py index c70197f7..20fefaad 100644 --- a/tests/test_upstream_pr.py +++ b/tests/test_upstream_pr.py @@ -122,7 +122,7 @@ def test_handle_github_message(self, mock_pr_from_github, mock_github): }, "mock_suffix", self.mock_config, - action=None, + None, ) mock_github.assert_called_with("mock_token", retry=5) self.assertEqual("Successful Call!", response) @@ -201,7 +201,6 @@ def test_github_issues( }, "open", self.mock_config, - action=None, ) self.mock_github_client.get_repo.assert_called_with("org/repo") self.mock_github_repo.get_pull.assert_called_with(number="1234") From a4b9bc3fc1bf2e9b4752880ad37ef0301825b62a Mon Sep 17 00:00:00 2001 From: Bala Sakabattula Date: Mon, 30 Mar 2026 14:41:08 +0530 Subject: [PATCH 3/3] adding a missing case --- tests/test_intermediary.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/tests/test_intermediary.py b/tests/test_intermediary.py index d5bb3d47..a6a4af6d 100644 --- a/tests/test_intermediary.py +++ b/tests/test_intermediary.py @@ -256,20 +256,22 @@ def test_from_github_pr_flat_topic_normalizes_suffix(self, mock_matcher): mock_matcher.return_value = "JIRA-1" flat = "github.pull_request" cases = ( - ("closed with merge", {"merged": True}, "closed", "merged"), - ("closed without merge", {"merged": False}, "closed", "closed"), - ("reopened", {}, "reopened", "reopened"), - ("opened", {}, "opened", "open"), - ("edited maps to open", {}, "edited", "open"), - ("missing action", {}, None, "open"), + ("closed with merge", {"merged": True}, "closed", "merged", flat), + ("closed without merge", {"merged": False}, "closed", "closed", flat), + ("reopened", {}, "reopened", "reopened", flat), + ("opened", {}, "opened", "open", flat), + ("edited maps to open", {}, "edited", "open", flat), + ("missing action flat topic", {}, None, "open", flat), + ("missing action preserves closed", {}, None, "closed", "closed"), + ("missing action preserves merged", {}, None, "merged", "merged"), ) - for name, pr_extra, action, expected in cases: + for name, pr_extra, action, expected, suffix in cases: with self.subTest(name): pr = {**self.mock_github_pr, **pr_extra} base_kw = dict( upstream="github", pr=pr, - suffix=flat, + suffix=suffix, config=self.mock_config, ) if action is not None: