diff --git a/docs/source/config-file.rst b/docs/source/config-file.rst index ad7fc3bd..192187e0 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', 'issue_types': ['Bug', 'Story']}` + * 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` @@ -192,17 +194,23 @@ 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'}` - * Sync when upstream PR gets merged. Attempts to transition JIRA ticket to CUSTOM_TRANSITION on upstream merge + * :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 + 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 + * Sync when upstream PR gets linked. Attempts to transition JIRA ticket to CUSTOM_TRANSITION on upstream link. * 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'}` - * 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_issue.py b/sync2jira/downstream_issue.py index 522e2d37..63cdadf8 100644 --- a/sync2jira/downstream_issue.py +++ b/sync2jira/downstream_issue.py @@ -1070,30 +1070,56 @@ 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"] + + # Normalize legacy True value to "Closed" + 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") + 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 ( + 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 90d21b03..2b3e830b 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 @@ -124,12 +125,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) @@ -140,25 +141,80 @@ 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] + pr_updates = pr.downstream.get("pr_updates") + if not pr_updates: + return - # Update the state - d_issue.change_status(client, existing, closed_status, pr) + for entry in 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..3a8fdfa5 100644 --- a/sync2jira/intermediary.py +++ b/sync2jira/intermediary.py @@ -155,6 +155,7 @@ def __init__( id_, suffix, match, + base_branch=None, downstream=None, ): self.source = source @@ -166,6 +167,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 +228,10 @@ def from_github(cls, upstream, pr, suffix, config, action=None): elif suffix not in lifecycle: suffix = "open" + base_branch = ( + pr["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 0de93559..c408d483 100644 --- a/tests/test_downstream_issue.py +++ b/tests/test_downstream_issue.py @@ -1334,6 +1334,167 @@ 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 independently sets up all pertinent state. + """ + mock_client.transitions.return_value = [{"name": "CLOSED", "id": "1234"}] + + scenarios = ( + # 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"], + "Bug", + "Closed", + False, + ), + # 4: Dict without 'transition' key is skipped + ( + "dict without transition key skipped", + [{"tags": {}}], + "Bug", + "Closed", + False, + ), + # 5: No issue_types filter — fires for any type + ( + "no issue_types filter fires for any type", + [{"transition": "CLOSED"}], + "Bug", + "Closed", + True, + ), + # 6: issue_types matches downstream type + ( + "issue_types filter match", + [{"transition": "CLOSED", "issue_types": ["Story", "Task"]}], + "Task", + "Closed", + True, + ), + # 7: issue_types does NOT match downstream type + ( + "issue_types filter no match", + [{"transition": "CLOSED", "issue_types": ["Story", "Task"]}], + "Bug", + "Closed", + False, + ), + # 8: 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, + ), + # 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"}], + "Bug", + "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 — normalized to "Closed" + ( + "transition value is True (normalized to Closed)", + [{"transition": True}], + "Bug", + "Closed", + True, + ), + # 13: transition value is False — no transition fires (non-string) + ( + "transition value is False", + [{"transition": False}], + "Bug", + "Closed", + False, + ), + ) + + 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() + 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 = downstream_status + + 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] + 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, + ) + 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..6c1d6daa 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,189 @@ 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_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 = "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 + "d_issue") + 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 = "Story" + 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_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.downstream = { + "pr_updates": [ + {"link_transition": "IN PROGRESS"}, + {"merge_transition": "MODIFIED"}, + ] + } + + 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_no_pr_updates(self, mock_d_issue): + """Test that update_transition returns silently when pr_updates is absent.""" + mock_client = MagicMock() + self.mock_pr.downstream = {} + + 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_empty_pr_updates(self, mock_d_issue): + """Test that update_transition returns silently when pr_updates is empty.""" + mock_client = MagicMock() + 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_not_called() + @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"])