From 6768b756f5873617e1be9f3c9c7b30c5e7c3f43b Mon Sep 17 00:00:00 2001 From: Ilanit Stein Date: Sat, 25 Apr 2026 22:42:15 +0300 Subject: [PATCH 1/6] Add branches and issue_types filters to transitions, dict-based fixVersion mapping, and tests Co-Authored-By: Claude Opus 4.6 --- sync2jira/downstream_issue.py | 51 +++++++++------ sync2jira/downstream_pr.py | 72 ++++++++++++++++++--- sync2jira/intermediary.py | 21 +++++- tests/test_downstream_issue.py | 101 +++++++++++++++++++++++++++++ tests/test_downstream_pr.py | 113 +++++++++++++++++++++++++++++++++ tests/test_intermediary.py | 67 ++++++++++++++++++- 6 files changed, 393 insertions(+), 32 deletions(-) diff --git a/sync2jira/downstream_issue.py b/sync2jira/downstream_issue.py index b835cb08..07c3f90a 100644 --- a/sync2jira/downstream_issue.py +++ b/sync2jira/downstream_issue.py @@ -1014,30 +1014,45 @@ def _update_transition(client, existing, issue): """ Helper function to update the transition of a downstream JIRA issue. + Supports an optional ``issue_types`` filter on transition entries in + ``issue_updates``. When present, the transition only fires if the + downstream JIRA issue's type is in the list. + :param jira.client.JIRA client: JIRA client :param jira.resource.Issue existing: Existing JIRA issue :param sync2jira.intermediary.Issue issue: Upstream issue :returns: Nothing """ - # If the user added a custom closed status, attempt to close the - # downstream JIRA ticket + for entry in issue.downstream.get("issue_updates", []): + if not isinstance(entry, dict) or "transition" not in entry: + continue - # First get the closed status from the config file - t = filter(lambda d: "transition" in d, issue.downstream.get("issue_updates", [])) - closed_status = next(t)["transition"] - if ( - closed_status is not True - and issue.status == "Closed" - and existing.fields.status.name.upper() != closed_status.upper() - ): - # Now we need to update the status of the JIRA issue - # First add a comment indicating the change (in case it doesn't go through) - hyperlink = f"[Upstream issue|{issue.url}]" - comment_body = f"{hyperlink} closed. Attempting transition to {closed_status}." - client.add_comment(existing, comment_body) - # Ensure that closed_status is a valid choice - # Find all possible transactions (i.e., change states) we could do - change_status(client, existing, closed_status, issue) + closed_status = entry["transition"] + + type_filters = entry.get("issue_types") + if type_filters is not None: + jira_type = existing.fields.issuetype.name + if jira_type not in type_filters: + log.info( + "Skipping issue transition '%s': issue type '%s' not in %s", + closed_status, + jira_type, + type_filters, + ) + continue + + if ( + closed_status is not True + and issue.status == "Closed" + and existing.fields.status.name.upper() != closed_status.upper() + ): + hyperlink = f"[Upstream issue|{issue.url}]" + comment_body = ( + f"{hyperlink} closed. Attempting transition to {closed_status}." + ) + client.add_comment(existing, comment_body) + change_status(client, existing, closed_status, issue) + return def _update_title(issue, existing): diff --git a/sync2jira/downstream_pr.py b/sync2jira/downstream_pr.py index bd8384d5..8042e383 100644 --- a/sync2jira/downstream_pr.py +++ b/sync2jira/downstream_pr.py @@ -17,6 +17,7 @@ # # Authors: Ralph Bean # Built-In Modules +import fnmatch import logging # 3rd Party Modules @@ -125,12 +126,12 @@ def update_jira_issue(existing, pr, client): remote_link = dict(url=pr.url, title=f"[PR] {pr.title}") d_issue.attach_link(client, existing, remote_link) - # Only synchronize link_transition for listings that op-in + # Only synchronize merge_transition for listings that opt-in if any("merge_transition" in item for item in updates) and "merged" in pr.suffix: log.info("Looking for new merged_transition") update_transition(client, existing, pr, "merge_transition") - # Only synchronize merge_transition for listings that op-in + # Only synchronize link_transition for listings that opt-in # and a link comment has been created if ( any("link_transition" in item for item in updates) @@ -141,25 +142,76 @@ def update_jira_issue(existing, pr, client): update_transition(client, existing, pr, "link_transition") +def _matches_transition_filters(transition_config, pr, existing): + """ + Check whether a transition config entry's optional filters match the + current PR and downstream JIRA issue. + + Supported filters: + - ``branches``: list of glob patterns matched against ``pr.base_branch`` + - ``issue_types``: list of JIRA issue type names matched against the + existing downstream issue's type + + :param dict transition_config: Single pr_updates entry + :param sync2jira.intermediary.PR pr: Upstream PR + :param jira.resources.Issue existing: Existing downstream JIRA issue + :returns: True if all filters pass (or no filters are specified) + :rtype: bool + """ + branch_filters = transition_config.get("branches") + if branch_filters is not None: + if not pr.base_branch or not any( + fnmatch.fnmatch(pr.base_branch, pattern) for pattern in branch_filters + ): + log.info( + "Skipping transition: branch '%s' does not match %s", + pr.base_branch, + branch_filters, + ) + return False + + type_filters = transition_config.get("issue_types") + if type_filters is not None: + jira_type = existing.fields.issuetype.name + if jira_type not in type_filters: + log.info( + "Skipping transition: issue type '%s' does not match %s", + jira_type, + type_filters, + ) + return False + + return True + + def update_transition(client, existing, pr, transition_type): """ Helper function to update the transition of a downstream JIRA issue. + Applies optional ``branches`` and ``issue_types`` filters from the + pr_updates config entry before executing the transition. + :param jira.client.JIRA client: JIRA client :param jira.resource.Issue existing: Existing JIRA issue :param sync2jira.intermediary.PR pr: Upstream issue :param string transition_type: Transition type (link vs merged) :returns: Nothing """ - # Get our closed status - closed_status = next( - filter(lambda d: transition_type in d, pr.downstream.get("pr_updates", {})) - )[transition_type] - - # Update the state - d_issue.change_status(client, existing, closed_status, pr) + for entry in pr.downstream.get("pr_updates", {}): + if transition_type not in entry: + continue + if not _matches_transition_filters(entry, pr, existing): + continue + d_issue.change_status(client, existing, entry[transition_type], pr) + log.info(f"Updated {transition_type} for issue {pr.title}") + return - log.info(f"Updated {transition_type} for issue {pr.title}") + log.info( + "No matching %s entry for PR %s (branch=%s)", + transition_type, + pr.title, + pr.base_branch, + ) def sync_with_jira(pr, config): diff --git a/sync2jira/intermediary.py b/sync2jira/intermediary.py index df91bcb6..7b277cf2 100644 --- a/sync2jira/intermediary.py +++ b/sync2jira/intermediary.py @@ -16,6 +16,7 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110.15.0 USA # # Authors: Ralph Bean +import fnmatch import re from typing import Optional @@ -155,6 +156,7 @@ def __init__( id_, suffix, match, + base_branch=None, downstream=None, ): self.source = source @@ -166,6 +168,7 @@ def __init__( # self.tags = tags # self.fixVersion = fixVersion self.priority = priority + self.base_branch = base_branch # JIRA treats utf-8 characters in ways we don't totally understand, so scrub content down to # simple ascii characters right from the start. @@ -226,6 +229,9 @@ def from_github(cls, upstream, pr, suffix, config, action=None): elif suffix not in lifecycle: suffix = "open" + # Extract the target branch from the PR payload + base_branch = pr.get("base", {}).get("ref") if isinstance(pr.get("base"), dict) else None + # Return our PR object return cls( source=upstream_source, @@ -247,6 +253,7 @@ def from_github(cls, upstream, pr, suffix, config, action=None): # upstream_id=issue['number'], suffix=suffix, match=match, + base_branch=base_branch, ) @@ -268,15 +275,23 @@ def map_fixVersion(mapping, issue): """ Helper function to perform any fixVersion mapping. + Supports two formats: + - String template: ``"Product XXX"`` — replaces ``XXX`` with the milestone value + - Dict lookup: ``{"0.9.0": "Product 8.1", ...}`` — maps milestone to a + specific fixVersion; unmapped milestones are left unchanged + :param Dict mapping: Mapping dict we are given :param Dict issue: Upstream issue object """ - # Get our fixVersion mapping fixVersion_map = next(filter(lambda d: "fixVersion" in d, mapping))["fixVersion"] - # Now update the fixVersion if issue["milestone"]: - issue["milestone"] = fixVersion_map.replace("XXX", issue["milestone"]) + if isinstance(fixVersion_map, dict): + issue["milestone"] = fixVersion_map.get( + issue["milestone"], issue["milestone"] + ) + else: + issue["milestone"] = fixVersion_map.replace("XXX", issue["milestone"]) JIRA_REFERENCE = re.compile(r"\bJIRA:\s*([A-Z][A-Z0-9]*-\d+)\b") diff --git a/tests/test_downstream_issue.py b/tests/test_downstream_issue.py index 18567578..292597f7 100644 --- a/tests/test_downstream_issue.py +++ b/tests/test_downstream_issue.py @@ -1291,6 +1291,107 @@ def test_update_transition_successful(self, mock_client): mock_client.transitions.assert_called_with(self.mock_downstream) mock_client.transition_issue.assert_called_with(self.mock_downstream, 1234) + @mock.patch("jira.client.JIRA") + def test_update_transition_issue_types_filter(self, mock_client): + """ + Table-driven test for _update_transition with issue_types filtering. + Each scenario is cumulative, adding one piece of state. + """ + mock_client.transitions.return_value = [ + {"name": "CLOSED", "id": "1234"} + ] + + scenarios = ( + # 1: Non-dict entry is skipped + ( + "non-dict entry skipped", + ["transition"], + "Bug", + "Closed", + False, + ), + # 2: Dict without 'transition' key is skipped + ( + "dict without transition key skipped", + [{"tags": {}}], + "Bug", + "Closed", + False, + ), + # 3: No issue_types filter — fires for any type + ( + "no issue_types filter fires for any type", + [{"transition": "CLOSED"}], + "Bug", + "Closed", + True, + ), + # 4: issue_types matches downstream type + ( + "issue_types filter match", + [{"transition": "CLOSED", "issue_types": ["Story", "Task"]}], + "Task", + "Closed", + True, + ), + # 5: issue_types does NOT match downstream type + ( + "issue_types filter no match", + [{"transition": "CLOSED", "issue_types": ["Story", "Task"]}], + "Bug", + "Closed", + False, + ), + # 6: Multiple entries — first skipped by type, second matches + ( + "multiple entries selects matching", + [ + {"transition": "MODIFIED", "issue_types": ["Bug"]}, + {"transition": "DEV COMPLETE", "issue_types": ["Story", "Task"]}, + ], + "Story", + "Closed", + True, + ), + # 7: Upstream issue not closed — no transition fires + ( + "upstream not closed", + [{"transition": "CLOSED"}], + "Bug", + "Open", + False, + ), + ) + + for name, issue_updates, jira_type, upstream_status, expect_transition in scenarios: + with self.subTest(name): + mock_client.reset_mock() + self.mock_issue.downstream = { + **self.mock_issue.downstream, + "issue_updates": issue_updates, + } + self.mock_issue.status = upstream_status + self.mock_downstream.fields.issuetype.name = jira_type + self.mock_downstream.fields.status.name = "Open" + + d._update_transition( + client=mock_client, + existing=self.mock_downstream, + issue=self.mock_issue, + ) + + if expect_transition: + mock_client.add_comment.assert_called_once() + comment_body = mock_client.add_comment.call_args[0][1] + expected_status = issue_updates[-1]["transition"] + self.assertIn( + f"Attempting transition to {expected_status}", + comment_body, + ) + mock_client.transitions.assert_called_with(self.mock_downstream) + else: + mock_client.add_comment.assert_not_called() + @mock.patch(PATH + "_comment_format") @mock.patch(PATH + "_comment_matching") @mock.patch("jira.client.JIRA") diff --git a/tests/test_downstream_pr.py b/tests/test_downstream_pr.py index baaa9c5d..07eaf7c6 100644 --- a/tests/test_downstream_pr.py +++ b/tests/test_downstream_pr.py @@ -362,6 +362,7 @@ def test_update_transition(self, mock_d_issue): """ # Set up return values mock_client = MagicMock() + self.mock_existing.fields.issuetype.name = "Bug" # Call the function d.update_transition( @@ -373,6 +374,118 @@ def test_update_transition(self, mock_d_issue): mock_client, self.mock_existing, "CUSTOM_TRANSITION1", self.mock_pr ) + @mock.patch(PATH + "d_issue") + def test_update_transition_branch_filter_match(self, mock_d_issue): + """Test merge_transition fires when branch matches the filter.""" + mock_client = MagicMock() + self.mock_existing.fields.issuetype.name = "Bug" + self.mock_pr.base_branch = "release-0.9" + self.mock_pr.downstream = { + "pr_updates": [ + {"merge_transition": "MODIFIED", "branches": ["release-*"]}, + ] + } + + d.update_transition( + mock_client, self.mock_existing, self.mock_pr, "merge_transition" + ) + + mock_d_issue.change_status.assert_called_with( + mock_client, self.mock_existing, "MODIFIED", self.mock_pr + ) + + @mock.patch(PATH + "d_issue") + def test_update_transition_branch_filter_no_match(self, mock_d_issue): + """Test merge_transition does NOT fire when branch doesn't match.""" + mock_client = MagicMock() + self.mock_existing.fields.issuetype.name = "Bug" + self.mock_pr.base_branch = "main" + self.mock_pr.downstream = { + "pr_updates": [ + {"merge_transition": "MODIFIED", "branches": ["release-*"]}, + ] + } + + d.update_transition( + mock_client, self.mock_existing, self.mock_pr, "merge_transition" + ) + + mock_d_issue.change_status.assert_not_called() + + @mock.patch(PATH + "d_issue") + def test_update_transition_issue_type_filter_match(self, mock_d_issue): + """Test transition fires when issue type matches the filter.""" + mock_client = MagicMock() + self.mock_existing.fields.issuetype.name = "Bug" + self.mock_pr.base_branch = "release-0.9" + self.mock_pr.downstream = { + "pr_updates": [ + { + "merge_transition": "MODIFIED", + "branches": ["release-*"], + "issue_types": ["Bug"], + }, + ] + } + + d.update_transition( + mock_client, self.mock_existing, self.mock_pr, "merge_transition" + ) + + mock_d_issue.change_status.assert_called_with( + mock_client, self.mock_existing, "MODIFIED", self.mock_pr + ) + + @mock.patch(PATH + "d_issue") + def test_update_transition_issue_type_filter_no_match(self, mock_d_issue): + """Test transition does NOT fire when issue type doesn't match.""" + mock_client = MagicMock() + self.mock_existing.fields.issuetype.name = "Story" + self.mock_pr.base_branch = "release-0.9" + self.mock_pr.downstream = { + "pr_updates": [ + { + "merge_transition": "MODIFIED", + "branches": ["release-*"], + "issue_types": ["Bug"], + }, + ] + } + + d.update_transition( + mock_client, self.mock_existing, self.mock_pr, "merge_transition" + ) + + mock_d_issue.change_status.assert_not_called() + + @mock.patch(PATH + "d_issue") + def test_update_transition_multiple_entries_selects_matching(self, mock_d_issue): + """Test that the correct transition is selected from multiple entries.""" + mock_client = MagicMock() + self.mock_existing.fields.issuetype.name = "Story" + self.mock_pr.base_branch = "main" + self.mock_pr.downstream = { + "pr_updates": [ + { + "merge_transition": "MODIFIED", + "branches": ["release-*"], + "issue_types": ["Bug"], + }, + { + "merge_transition": "Dev Complete", + "issue_types": ["Story", "Task"], + }, + ] + } + + d.update_transition( + mock_client, self.mock_existing, self.mock_pr, "merge_transition" + ) + + mock_d_issue.change_status.assert_called_with( + mock_client, self.mock_existing, "Dev Complete", self.mock_pr + ) + @mock.patch(PATH + "update_jira") @mock.patch(PATH + "d_issue") def test_sync_with_jira_create_pr_issue_enabled( diff --git a/tests/test_intermediary.py b/tests/test_intermediary.py index a6a4af6d..dd5dbbe3 100644 --- a/tests/test_intermediary.py +++ b/tests/test_intermediary.py @@ -361,4 +361,69 @@ def test_matcher(self): actual = i.matcher(content, comments) self.assertEqual(expected, actual) - # TODO: Add new tests from PR + def test_map_fixVersion(self): + """ + Table-driven test for map_fixVersion covering both string-template + and dict-based lookup formats. + """ + scenarios = ( + # 1: String template (existing XXX replacement) + ( + "string template", + "Product XXX", + "1.0", + "Product 1.0", + ), + # 2: Dict lookup — known key + ( + "dict lookup known key", + {"v1.0": "Release 1.0", "v2.0": "Release 2.0"}, + "v1.0", + "Release 1.0", + ), + # 3: Dict lookup — unknown key left unchanged + ( + "dict lookup unknown key unchanged", + {"v1.0": "Release 1.0"}, + "v3.0", + "v3.0", + ), + # 4: Dict lookup — empty dict leaves milestone unchanged + ( + "dict lookup empty dict unchanged", + {}, + "v1.0", + "v1.0", + ), + # 5: None milestone — no mapping applied + ( + "none milestone no mapping", + {"v1.0": "Release 1.0"}, + None, + None, + ), + ) + + for name, fixversion_map, milestone, expected in scenarios: + with self.subTest(name): + mapping = [{"fixVersion": fixversion_map}] + issue = {"milestone": milestone} + i.map_fixVersion(mapping, issue) + self.assertEqual(issue["milestone"], expected) + + def test_mapping_github_dict_fixVersion(self): + """ + End-to-end test: dict-based fixVersion mapping through Issue.from_github. + """ + self.mock_config["sync2jira"]["map"]["github"]["github"] = { + "mock_downstream": "mock_key", + "mapping": [{"fixVersion": {"mock_milestone": "Mapped Version 1.0"}}], + } + self.mock_github_issue["state"] = "closed" + + response = i.Issue.from_github( + upstream="github", issue=self.mock_github_issue, config=self.mock_config + ) + + self.checkResponseFields(response) + self.assertEqual(response.fixVersion, ["Mapped Version 1.0"]) From b3565a537f3a7ec7ff9b3e545f2c485de4fbc73a Mon Sep 17 00:00:00 2001 From: Ilanit Stein Date: Sun, 26 Apr 2026 09:27:32 +0300 Subject: [PATCH 2/6] Fix lint and formatting: remove unused fnmatch import, apply Black Co-Authored-By: Claude Opus 4.6 --- sync2jira/intermediary.py | 5 +++-- tests/test_downstream_issue.py | 12 ++++++++---- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/sync2jira/intermediary.py b/sync2jira/intermediary.py index 7b277cf2..21b12a55 100644 --- a/sync2jira/intermediary.py +++ b/sync2jira/intermediary.py @@ -16,7 +16,6 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110.15.0 USA # # Authors: Ralph Bean -import fnmatch import re from typing import Optional @@ -230,7 +229,9 @@ def from_github(cls, upstream, pr, suffix, config, action=None): suffix = "open" # Extract the target branch from the PR payload - base_branch = pr.get("base", {}).get("ref") if isinstance(pr.get("base"), dict) else None + base_branch = ( + pr.get("base", {}).get("ref") if isinstance(pr.get("base"), dict) else None + ) # Return our PR object return cls( diff --git a/tests/test_downstream_issue.py b/tests/test_downstream_issue.py index 292597f7..edbb3bed 100644 --- a/tests/test_downstream_issue.py +++ b/tests/test_downstream_issue.py @@ -1297,9 +1297,7 @@ def test_update_transition_issue_types_filter(self, mock_client): Table-driven test for _update_transition with issue_types filtering. Each scenario is cumulative, adding one piece of state. """ - mock_client.transitions.return_value = [ - {"name": "CLOSED", "id": "1234"} - ] + mock_client.transitions.return_value = [{"name": "CLOSED", "id": "1234"}] scenarios = ( # 1: Non-dict entry is skipped @@ -1363,7 +1361,13 @@ def test_update_transition_issue_types_filter(self, mock_client): ), ) - for name, issue_updates, jira_type, upstream_status, expect_transition in scenarios: + for ( + name, + issue_updates, + jira_type, + upstream_status, + expect_transition, + ) in scenarios: with self.subTest(name): mock_client.reset_mock() self.mock_issue.downstream = { From 041f9e64e4c8f7ba48fd6a184af63e16c39d9ff6 Mon Sep 17 00:00:00 2001 From: Ilanit Stein Date: Fri, 1 May 2026 17:22:45 +0300 Subject: [PATCH 3/6] Address PR #460 review feedback - Guard noisy log in update_transition when pr_updates is absent - Simplify base_branch extraction with dict reference instead of .get() - Split PR filter tests into separate units for _matches_transition_filters and update_transition - Add missing issue test scenarios: absent/empty issue_updates, none-match multiple entries, already-matching status, transition=True - Fix misleading "cumulative" comment in issue test docstring - Document branches, issue_types filters and dict-based fixVersion mapping Co-Authored-By: Claude Opus 4.6 --- docs/source/config-file.rst | 21 ++-- sync2jira/downstream_pr.py | 6 +- sync2jira/intermediary.py | 3 +- tests/test_downstream_issue.py | 87 +++++++++++++---- tests/test_downstream_pr.py | 173 +++++++++++++++++++++++---------- 5 files changed, 210 insertions(+), 80 deletions(-) diff --git a/docs/source/config-file.rst b/docs/source/config-file.rst index ad7fc3bd..7bd5fcf9 100644 --- a/docs/source/config-file.rst +++ b/docs/source/config-file.rst @@ -167,8 +167,10 @@ The config file is made up of multiple parts * Sync description * :code:`'title'` * Sync title - * :code:`{'transition': True/'CUSTOM_TRANSITION'}` - * Sync status (open/closed), Sync only status/Attempt to transition JIRA ticket to CUSTOM_TRANSITION on upstream closure + * :code:`{'transition': 'CUSTOM_TRANSITION'}` + * Sync status (open/closed). Attempt to transition JIRA ticket to CUSTOM_TRANSITION on upstream closure. + * :code:`{'transition': 'CUSTOM_TRANSITION', 'issue_types': ['Bug', 'Story']}` + * Same as above, but the transition only fires when the downstream JIRA issue type is in the list. * :code:`{'on_close': {'apply_labels': ['label', ...]}}` * When the upstream issue is closed, apply additional labels on the corresponding Jira ticket. * :code:`github_markdown` @@ -193,16 +195,23 @@ The config file is made up of multiple parts downstream components. You can add the following to the :code:`pr_updates` array: * :code:`{'merge_transition': 'CUSTOM_TRANSITION'}` - * Sync when upstream PR gets merged. Attempts to transition JIRA ticket to CUSTOM_TRANSITION on upstream merge + * Sync when upstream PR gets merged. Attempts to transition JIRA ticket to CUSTOM_TRANSITION on upstream merge. * :code:`{'link_transition': 'CUSTOM_TRANSITION'}` - * Sync when upstream PR gets linked. Attempts to transition JIRA ticket to CUSTOM_TRANSITION on upstream link + * Sync when upstream PR gets linked. Attempts to transition JIRA ticket to CUSTOM_TRANSITION on upstream link. + * :code:`'branches': ['release-*', 'main']` + * Optional filter (glob patterns). The transition only fires when the PR's target branch matches one of the patterns. + * :code:`'issue_types': ['Bug', 'Story']` + * Optional filter. The transition only fires when the downstream JIRA issue type is in the list. * You can add the following to the mapping array. This array will map an upstream field to the downstream counterpart with XXX replaced. * :code:`{'fixVersion': 'Test XXX'}` - * Maps upstream milestone (suppose it's called 'milestone') to downstream fixVersion with a mapping (for our - example it would be 'Test milestone') + * String template format. Maps upstream milestone (suppose it's called 'milestone') to downstream fixVersion + with a mapping (for our example it would be 'Test milestone'). + * :code:`{'fixVersion': {'0.9.0': 'Product 8.1', '1.0.0': 'Product 9.0'}}` + * Dict lookup format. Maps specific upstream milestones to specific downstream fixVersions. + Milestones not present in the dict are left unchanged. * It is strongly encouraged for teams to use the :code:`owner` field. If configured, owners will be alerted if Sync2Jira finds duplicate downstream issues. Further the owner will be used as a default in case the program is unable to find a diff --git a/sync2jira/downstream_pr.py b/sync2jira/downstream_pr.py index 8042e383..697d4db5 100644 --- a/sync2jira/downstream_pr.py +++ b/sync2jira/downstream_pr.py @@ -197,7 +197,11 @@ def update_transition(client, existing, pr, transition_type): :param string transition_type: Transition type (link vs merged) :returns: Nothing """ - for entry in pr.downstream.get("pr_updates", {}): + pr_updates = pr.downstream.get("pr_updates") + if not pr_updates: + return + + for entry in pr_updates: if transition_type not in entry: continue if not _matches_transition_filters(entry, pr, existing): diff --git a/sync2jira/intermediary.py b/sync2jira/intermediary.py index 21b12a55..3a8fdfa5 100644 --- a/sync2jira/intermediary.py +++ b/sync2jira/intermediary.py @@ -228,9 +228,8 @@ def from_github(cls, upstream, pr, suffix, config, action=None): elif suffix not in lifecycle: suffix = "open" - # Extract the target branch from the PR payload base_branch = ( - pr.get("base", {}).get("ref") if isinstance(pr.get("base"), dict) else None + pr["base"].get("ref") if isinstance(pr.get("base"), dict) else None ) # Return our PR object diff --git a/tests/test_downstream_issue.py b/tests/test_downstream_issue.py index 56f3670b..c9b14519 100644 --- a/tests/test_downstream_issue.py +++ b/tests/test_downstream_issue.py @@ -1295,12 +1295,28 @@ def test_update_transition_successful(self, mock_client): def test_update_transition_issue_types_filter(self, mock_client): """ Table-driven test for _update_transition with issue_types filtering. - Each scenario is cumulative, adding one piece of state. + Each scenario independently sets up all pertinent state. """ mock_client.transitions.return_value = [{"name": "CLOSED", "id": "1234"}] scenarios = ( - # 1: Non-dict entry is skipped + # 1: issue_updates key absent from downstream — no transition + ( + "issue_updates key absent", + None, + "Bug", + "Closed", + False, + ), + # 2: issue_updates is an empty list — no transition + ( + "issue_updates empty list", + [], + "Bug", + "Closed", + False, + ), + # 3: Non-dict entry is skipped ( "non-dict entry skipped", ["transition"], @@ -1308,7 +1324,7 @@ def test_update_transition_issue_types_filter(self, mock_client): "Closed", False, ), - # 2: Dict without 'transition' key is skipped + # 4: Dict without 'transition' key is skipped ( "dict without transition key skipped", [{"tags": {}}], @@ -1316,7 +1332,7 @@ def test_update_transition_issue_types_filter(self, mock_client): "Closed", False, ), - # 3: No issue_types filter — fires for any type + # 5: No issue_types filter — fires for any type ( "no issue_types filter fires for any type", [{"transition": "CLOSED"}], @@ -1324,7 +1340,7 @@ def test_update_transition_issue_types_filter(self, mock_client): "Closed", True, ), - # 4: issue_types matches downstream type + # 6: issue_types matches downstream type ( "issue_types filter match", [{"transition": "CLOSED", "issue_types": ["Story", "Task"]}], @@ -1332,7 +1348,7 @@ def test_update_transition_issue_types_filter(self, mock_client): "Closed", True, ), - # 5: issue_types does NOT match downstream type + # 7: issue_types does NOT match downstream type ( "issue_types filter no match", [{"transition": "CLOSED", "issue_types": ["Story", "Task"]}], @@ -1340,7 +1356,7 @@ def test_update_transition_issue_types_filter(self, mock_client): "Closed", False, ), - # 6: Multiple entries — first skipped by type, second matches + # 8: Multiple entries — first skipped by type, second matches ( "multiple entries selects matching", [ @@ -1351,7 +1367,18 @@ def test_update_transition_issue_types_filter(self, mock_client): "Closed", True, ), - # 7: Upstream issue not closed — no transition fires + # 9: Multiple entries, none match — no transition + ( + "multiple entries none match", + [ + {"transition": "MODIFIED", "issue_types": ["Bug"]}, + {"transition": "DEV COMPLETE", "issue_types": ["Task"]}, + ], + "Story", + "Closed", + False, + ), + # 10: Upstream issue not closed — no transition fires ( "upstream not closed", [{"transition": "CLOSED"}], @@ -1359,24 +1386,44 @@ def test_update_transition_issue_types_filter(self, mock_client): "Open", False, ), + # 11: Downstream status already matches transition target — no transition + ( + "downstream status already matches", + [{"transition": "CLOSED"}], + "Bug", + "Closed", + False, + "CLOSED", + ), + # 12: transition value is True — no transition fires + ( + "transition value is True", + [{"transition": True}], + "Bug", + "Closed", + False, + ), ) - for ( - name, - issue_updates, - jira_type, - upstream_status, - expect_transition, - ) in scenarios: + for scenario in scenarios: + name = scenario[0] + issue_updates = scenario[1] + jira_type = scenario[2] + upstream_status = scenario[3] + expect_transition = scenario[4] + downstream_status = scenario[5] if len(scenario) > 5 else "Open" + with self.subTest(name): mock_client.reset_mock() - self.mock_issue.downstream = { - **self.mock_issue.downstream, - "issue_updates": issue_updates, - } + downstream = {**self.mock_issue.downstream} + if issue_updates is None: + downstream.pop("issue_updates", None) + else: + downstream["issue_updates"] = issue_updates + self.mock_issue.downstream = downstream self.mock_issue.status = upstream_status self.mock_downstream.fields.issuetype.name = jira_type - self.mock_downstream.fields.status.name = "Open" + self.mock_downstream.fields.status.name = downstream_status d._update_transition( client=mock_client, diff --git a/tests/test_downstream_pr.py b/tests/test_downstream_pr.py index 07eaf7c6..6c1d6daa 100644 --- a/tests/test_downstream_pr.py +++ b/tests/test_downstream_pr.py @@ -374,15 +374,113 @@ def test_update_transition(self, mock_d_issue): mock_client, self.mock_existing, "CUSTOM_TRANSITION1", self.mock_pr ) + def test_matches_transition_filters(self): + """Table-driven test for _matches_transition_filters as a separate unit.""" + scenarios = ( + ( + "no filters — passes", + {"merge_transition": "MODIFIED"}, + "main", + "Bug", + True, + ), + ( + "branch matches glob", + {"merge_transition": "MODIFIED", "branches": ["release-*"]}, + "release-0.9", + "Bug", + True, + ), + ( + "branch does not match glob", + {"merge_transition": "MODIFIED", "branches": ["release-*"]}, + "main", + "Bug", + False, + ), + ( + "issue type matches", + {"merge_transition": "MODIFIED", "issue_types": ["Bug"]}, + "main", + "Bug", + True, + ), + ( + "issue type does not match", + {"merge_transition": "MODIFIED", "issue_types": ["Bug"]}, + "main", + "Story", + False, + ), + ( + "both filters match", + { + "merge_transition": "MODIFIED", + "branches": ["release-*"], + "issue_types": ["Bug"], + }, + "release-0.9", + "Bug", + True, + ), + ( + "branch matches but issue type does not", + { + "merge_transition": "MODIFIED", + "branches": ["release-*"], + "issue_types": ["Bug"], + }, + "release-0.9", + "Story", + False, + ), + ( + "issue type matches but branch does not", + { + "merge_transition": "MODIFIED", + "branches": ["release-*"], + "issue_types": ["Bug"], + }, + "main", + "Bug", + False, + ), + ( + "base_branch is None with branch filter", + {"merge_transition": "MODIFIED", "branches": ["release-*"]}, + None, + "Bug", + False, + ), + ) + + for name, entry, base_branch, jira_type, expected in scenarios: + with self.subTest(name): + self.mock_pr.base_branch = base_branch + self.mock_existing.fields.issuetype.name = jira_type + + result = d._matches_transition_filters( + entry, self.mock_pr, self.mock_existing + ) + self.assertEqual(result, expected) + @mock.patch(PATH + "d_issue") - def test_update_transition_branch_filter_match(self, mock_d_issue): - """Test merge_transition fires when branch matches the filter.""" + def test_update_transition_selects_first_matching_entry(self, mock_d_issue): + """Test that update_transition iterates entries and selects the first match.""" mock_client = MagicMock() - self.mock_existing.fields.issuetype.name = "Bug" - self.mock_pr.base_branch = "release-0.9" + self.mock_existing.fields.issuetype.name = "Story" + self.mock_pr.base_branch = "main" self.mock_pr.downstream = { "pr_updates": [ - {"merge_transition": "MODIFIED", "branches": ["release-*"]}, + { + "merge_transition": "MODIFIED", + "branches": ["release-*"], + "issue_types": ["Bug"], + }, + { + "merge_transition": "Dev Complete", + "issue_types": ["Story", "Task"], + }, ] } @@ -391,18 +489,21 @@ def test_update_transition_branch_filter_match(self, mock_d_issue): ) mock_d_issue.change_status.assert_called_with( - mock_client, self.mock_existing, "MODIFIED", self.mock_pr + mock_client, self.mock_existing, "Dev Complete", self.mock_pr ) @mock.patch(PATH + "d_issue") - def test_update_transition_branch_filter_no_match(self, mock_d_issue): - """Test merge_transition does NOT fire when branch doesn't match.""" + def test_update_transition_no_matching_entry(self, mock_d_issue): + """Test that no transition fires when no entries match.""" mock_client = MagicMock() - self.mock_existing.fields.issuetype.name = "Bug" + self.mock_existing.fields.issuetype.name = "Story" self.mock_pr.base_branch = "main" self.mock_pr.downstream = { "pr_updates": [ - {"merge_transition": "MODIFIED", "branches": ["release-*"]}, + { + "merge_transition": "MODIFIED", + "branches": ["release-*"], + }, ] } @@ -413,18 +514,14 @@ def test_update_transition_branch_filter_no_match(self, mock_d_issue): mock_d_issue.change_status.assert_not_called() @mock.patch(PATH + "d_issue") - def test_update_transition_issue_type_filter_match(self, mock_d_issue): - """Test transition fires when issue type matches the filter.""" + def test_update_transition_skips_entry_without_transition_type(self, mock_d_issue): + """Test that entries without the requested transition type are skipped.""" mock_client = MagicMock() self.mock_existing.fields.issuetype.name = "Bug" - self.mock_pr.base_branch = "release-0.9" self.mock_pr.downstream = { "pr_updates": [ - { - "merge_transition": "MODIFIED", - "branches": ["release-*"], - "issue_types": ["Bug"], - }, + {"link_transition": "IN PROGRESS"}, + {"merge_transition": "MODIFIED"}, ] } @@ -437,20 +534,10 @@ def test_update_transition_issue_type_filter_match(self, mock_d_issue): ) @mock.patch(PATH + "d_issue") - def test_update_transition_issue_type_filter_no_match(self, mock_d_issue): - """Test transition does NOT fire when issue type doesn't match.""" + def test_update_transition_no_pr_updates(self, mock_d_issue): + """Test that update_transition returns silently when pr_updates is absent.""" mock_client = MagicMock() - self.mock_existing.fields.issuetype.name = "Story" - self.mock_pr.base_branch = "release-0.9" - self.mock_pr.downstream = { - "pr_updates": [ - { - "merge_transition": "MODIFIED", - "branches": ["release-*"], - "issue_types": ["Bug"], - }, - ] - } + self.mock_pr.downstream = {} d.update_transition( mock_client, self.mock_existing, self.mock_pr, "merge_transition" @@ -459,32 +546,16 @@ def test_update_transition_issue_type_filter_no_match(self, mock_d_issue): mock_d_issue.change_status.assert_not_called() @mock.patch(PATH + "d_issue") - def test_update_transition_multiple_entries_selects_matching(self, mock_d_issue): - """Test that the correct transition is selected from multiple entries.""" + def test_update_transition_empty_pr_updates(self, mock_d_issue): + """Test that update_transition returns silently when pr_updates is empty.""" mock_client = MagicMock() - self.mock_existing.fields.issuetype.name = "Story" - self.mock_pr.base_branch = "main" - self.mock_pr.downstream = { - "pr_updates": [ - { - "merge_transition": "MODIFIED", - "branches": ["release-*"], - "issue_types": ["Bug"], - }, - { - "merge_transition": "Dev Complete", - "issue_types": ["Story", "Task"], - }, - ] - } + self.mock_pr.downstream = {"pr_updates": []} d.update_transition( mock_client, self.mock_existing, self.mock_pr, "merge_transition" ) - mock_d_issue.change_status.assert_called_with( - mock_client, self.mock_existing, "Dev Complete", self.mock_pr - ) + mock_d_issue.change_status.assert_not_called() @mock.patch(PATH + "update_jira") @mock.patch(PATH + "d_issue") From 316ee3da5d4f1f9a1e0f2797031a1e0242585954 Mon Sep 17 00:00:00 2001 From: Ilanit Stein Date: Sun, 3 May 2026 09:50:04 +0300 Subject: [PATCH 4/6] Harden transition guard and clarify pr_updates docs - Replace `closed_status is not True` with `isinstance(closed_status, str)` to safely skip any non-string value (True, False, None, etc.) - Show branches/issue_types as keys in the same dict in pr_updates docs - Add test scenario for transition: False Co-Authored-By: Claude Opus 4.6 --- docs/source/config-file.rst | 8 ++++---- sync2jira/downstream_issue.py | 2 +- tests/test_downstream_issue.py | 8 ++++++++ 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/docs/source/config-file.rst b/docs/source/config-file.rst index 7bd5fcf9..39a9b36b 100644 --- a/docs/source/config-file.rst +++ b/docs/source/config-file.rst @@ -198,10 +198,10 @@ The config file is made up of multiple parts * Sync when upstream PR gets merged. Attempts to transition JIRA ticket to CUSTOM_TRANSITION on upstream merge. * :code:`{'link_transition': 'CUSTOM_TRANSITION'}` * Sync when upstream PR gets linked. Attempts to transition JIRA ticket to CUSTOM_TRANSITION on upstream link. - * :code:`'branches': ['release-*', 'main']` - * Optional filter (glob patterns). The transition only fires when the PR's target branch matches one of the patterns. - * :code:`'issue_types': ['Bug', 'Story']` - * Optional filter. The transition only fires when the downstream JIRA issue type is in the list. + * :code:`{'merge_transition': 'MODIFIED', 'branches': ['release-*', 'main'], 'issue_types': ['Bug', 'Story']}` + * Optional filters added to the same dict as the transition. ``branches`` accepts glob patterns and restricts + the transition to PRs whose target branch matches. ``issue_types`` restricts it to matching downstream JIRA + issue types. Both filters are optional and can be used independently or together. * You can add the following to the mapping array. This array will map an upstream field to the downstream counterpart with XXX replaced. diff --git a/sync2jira/downstream_issue.py b/sync2jira/downstream_issue.py index 804b9000..89ca4ba6 100644 --- a/sync2jira/downstream_issue.py +++ b/sync2jira/downstream_issue.py @@ -1098,7 +1098,7 @@ def _update_transition(client, existing, issue): continue if ( - closed_status is not True + isinstance(closed_status, str) and issue.status == "Closed" and existing.fields.status.name.upper() != closed_status.upper() ): diff --git a/tests/test_downstream_issue.py b/tests/test_downstream_issue.py index c9b14519..fadcc8cb 100644 --- a/tests/test_downstream_issue.py +++ b/tests/test_downstream_issue.py @@ -1403,6 +1403,14 @@ def test_update_transition_issue_types_filter(self, mock_client): "Closed", False, ), + # 13: transition value is False — no transition fires (non-string) + ( + "transition value is False", + [{"transition": False}], + "Bug", + "Closed", + False, + ), ) for scenario in scenarios: From c000887909c69c949bdd77b73372d05e1b0f77d6 Mon Sep 17 00:00:00 2001 From: Ilanit Stein Date: Fri, 8 May 2026 18:42:41 +0300 Subject: [PATCH 5/6] Address PR #460 second review: consolidate docs, normalize transition True - Consolidate merge_transition and issue_updates transition doc entries into single entries showing optional filters, per reviewer suggestion - Fix mapping array description wording - Normalize legacy transition: True to "Closed" (confirmed by Ralph) - Update test expectation for True normalization Co-Authored-By: Claude Opus 4.6 --- docs/source/config-file.rst | 17 ++++++++--------- sync2jira/downstream_issue.py | 9 +++++++-- tests/test_downstream_issue.py | 9 +++++---- 3 files changed, 20 insertions(+), 15 deletions(-) diff --git a/docs/source/config-file.rst b/docs/source/config-file.rst index 39a9b36b..f49520f5 100644 --- a/docs/source/config-file.rst +++ b/docs/source/config-file.rst @@ -167,10 +167,10 @@ The config file is made up of multiple parts * Sync description * :code:`'title'` * Sync title - * :code:`{'transition': 'CUSTOM_TRANSITION'}` - * Sync status (open/closed). Attempt to transition JIRA ticket to CUSTOM_TRANSITION on upstream closure. * :code:`{'transition': 'CUSTOM_TRANSITION', 'issue_types': ['Bug', 'Story']}` - * Same as above, but the transition only fires when the downstream JIRA issue type is in the list. + * Sync status (open/closed). Attempt to transition JIRA ticket to CUSTOM_TRANSITION on upstream closure. + * ``issue_types`` is optional and may be omitted. When present, the transition only fires if the + downstream JIRA issue type is in the list. * :code:`{'on_close': {'apply_labels': ['label', ...]}}` * When the upstream issue is closed, apply additional labels on the corresponding Jira ticket. * :code:`github_markdown` @@ -194,17 +194,16 @@ The config file is made up of multiple parts * You can add your projects here. The 'project' field is associated with downstream JIRA projects, and 'component' with downstream components. You can add the following to the :code:`pr_updates` array: - * :code:`{'merge_transition': 'CUSTOM_TRANSITION'}` + * :code:`{'merge_transition': 'MODIFIED', 'branches': ['release-*', 'main'], 'issue_types': ['Bug', 'Story']}` * Sync when upstream PR gets merged. Attempts to transition JIRA ticket to CUSTOM_TRANSITION on upstream merge. + * ``branches`` and ``issue_types`` are optional and either may be omitted. ``branches`` accepts glob patterns + and restricts the transition to PRs whose target branch matches. ``issue_types`` restricts it to matching + downstream JIRA issue types. * :code:`{'link_transition': 'CUSTOM_TRANSITION'}` * Sync when upstream PR gets linked. Attempts to transition JIRA ticket to CUSTOM_TRANSITION on upstream link. - * :code:`{'merge_transition': 'MODIFIED', 'branches': ['release-*', 'main'], 'issue_types': ['Bug', 'Story']}` - * Optional filters added to the same dict as the transition. ``branches`` accepts glob patterns and restricts - the transition to PRs whose target branch matches. ``issue_types`` restricts it to matching downstream JIRA - issue types. Both filters are optional and can be used independently or together. * You can add the following to the mapping array. This array will map an upstream field to the downstream counterpart - with XXX replaced. + using either a template or a mapping table. * :code:`{'fixVersion': 'Test XXX'}` * String template format. Maps upstream milestone (suppose it's called 'milestone') to downstream fixVersion diff --git a/sync2jira/downstream_issue.py b/sync2jira/downstream_issue.py index 89ca4ba6..566fb227 100644 --- a/sync2jira/downstream_issue.py +++ b/sync2jira/downstream_issue.py @@ -1085,6 +1085,12 @@ def _update_transition(client, existing, issue): closed_status = entry["transition"] + # Normalize legacy True value to "Closed" + if closed_status is True: + closed_status = "Closed" + if not isinstance(closed_status, str): + continue + type_filters = entry.get("issue_types") if type_filters is not None: jira_type = existing.fields.issuetype.name @@ -1098,8 +1104,7 @@ def _update_transition(client, existing, issue): continue if ( - isinstance(closed_status, str) - and issue.status == "Closed" + issue.status == "Closed" and existing.fields.status.name.upper() != closed_status.upper() ): hyperlink = f"[Upstream issue|{issue.url}]" diff --git a/tests/test_downstream_issue.py b/tests/test_downstream_issue.py index fadcc8cb..734cb76b 100644 --- a/tests/test_downstream_issue.py +++ b/tests/test_downstream_issue.py @@ -1395,13 +1395,13 @@ def test_update_transition_issue_types_filter(self, mock_client): False, "CLOSED", ), - # 12: transition value is True — no transition fires + # 12: transition value is True — normalized to "Closed" ( - "transition value is True", + "transition value is True (normalized to Closed)", [{"transition": True}], "Bug", "Closed", - False, + True, ), # 13: transition value is False — no transition fires (non-string) ( @@ -1442,7 +1442,8 @@ def test_update_transition_issue_types_filter(self, mock_client): if expect_transition: mock_client.add_comment.assert_called_once() comment_body = mock_client.add_comment.call_args[0][1] - expected_status = issue_updates[-1]["transition"] + raw_status = issue_updates[-1]["transition"] + expected_status = "Closed" if raw_status is True else raw_status self.assertIn( f"Attempting transition to {expected_status}", comment_body, From a703e5a367375a08da2819ea91776d1550140276 Mon Sep 17 00:00:00 2001 From: Ilanit Stein Date: Tue, 12 May 2026 18:18:58 +0300 Subject: [PATCH 6/6] Address PR #460 third review: fix doc example and add malformed config warning Use CUSTOM_TRANSITION placeholder in pr_updates doc example for consistency, and log a warning when a non-string transition value is encountered in issue_updates config. Co-Authored-By: Claude Opus 4.6 --- docs/source/config-file.rst | 2 +- sync2jira/downstream_issue.py | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/docs/source/config-file.rst b/docs/source/config-file.rst index f49520f5..192187e0 100644 --- a/docs/source/config-file.rst +++ b/docs/source/config-file.rst @@ -194,7 +194,7 @@ The config file is made up of multiple parts * You can add your projects here. The 'project' field is associated with downstream JIRA projects, and 'component' with downstream components. You can add the following to the :code:`pr_updates` array: - * :code:`{'merge_transition': 'MODIFIED', 'branches': ['release-*', 'main'], 'issue_types': ['Bug', 'Story']}` + * :code:`{'merge_transition': 'CUSTOM_TRANSITION', 'branches': ['release-*', 'main'], 'issue_types': ['Bug', 'Story']}` * Sync when upstream PR gets merged. Attempts to transition JIRA ticket to CUSTOM_TRANSITION on upstream merge. * ``branches`` and ``issue_types`` are optional and either may be omitted. ``branches`` accepts glob patterns and restricts the transition to PRs whose target branch matches. ``issue_types`` restricts it to matching diff --git a/sync2jira/downstream_issue.py b/sync2jira/downstream_issue.py index 566fb227..bf373804 100644 --- a/sync2jira/downstream_issue.py +++ b/sync2jira/downstream_issue.py @@ -1089,6 +1089,12 @@ def _update_transition(client, existing, issue): if closed_status is True: closed_status = "Closed" if not isinstance(closed_status, str): + log.warning( + "Ignoring malformed transition value %r (expected a string) in " + "issue_updates config for %s", + closed_status, + existing.key, + ) continue type_filters = entry.get("issue_types")