Repository: release-engineering/Sync2Jira

Issue:
Add ability to sync JIRA issue type with GH issue type
From this conversation in slack: https://redhat-internal.slack.com/archives/C058M9PBGEM/p1737495007203029

Currently, the only way for sync2jira to give jira a type is from GH labels. GH now has a a a type field that we'd like to be able to sync to jira on.

Discussion:

Solution patch:
diff --git a/sync2jira/downstream_issue.py b/sync2jira/downstream_issue.py
index d1df8e3..dc2ee9d 100644
--- a/sync2jira/downstream_issue.py
+++ b/sync2jira/downstream_issue.py
@@ -425,6 +425,14 @@ def assign_user(
                 downstream.update({"assignee": {"name": match_name}})
                 return
 
+    if issue.assignee:
+        log.warning(
+            "Unable to assign %s from upstream assignees %s in %s",
+            downstream.key,
+            str([a.get("fullname", "<no-fullname>") for a in issue.assignee]),
+            issue.url,
+        )
+
     # No downstream match for the upstream assignee; if there is a configured
     # owner for the project, assign it to them.
     owner = issue.downstream.get("owner")
@@ -432,12 +440,6 @@ def assign_user(
         client.assign_issue(downstream.id, owner)
         log.info("Assigned %s to owner: %s", downstream.key, owner)
         return
-    log.info(
-        "Unable to assign %s from upstream assignees %s in %s",
-        downstream.key,
-        [a["fullname"] for a in issue.assignee],
-        issue.url,
-    )
 
 
 def change_status(client, downstream, status, issue: Union[Issue, PR]):
@@ -484,9 +486,19 @@ def change_status(client, downstream, status, issue: Union[Issue, PR]):
 
 def _get_preferred_issue_types(config, issue):
     """
-    Consider the configuration file and issue data to
-    figure out the right issue type to file. Used when
-    creating a new issue in Jira.
+    Determine the appropriate issue type to specify when creating the
+    downstream (Jira) issue.  In order of preference:
+     - the issue type(s) from the mapping in the configuration file (if
+        present), selected based on the upstream "tags" (labels)
+     - the default issue type configured for the project (if any)
+     - the upstream issue type (if any)
+     - "Story" if the issue title contains "RFE"
+     - otherwise, "Bug".
+
+    In all cases, a list of one item is returned, except when the upstream
+    issue has multiple tags which match multiple entries in the configured
+    mapping, in which case multiple entries are returned, sorted in ascending
+    lexographical order.
 
     :param Dict config: Config dict
     :param sync2jira.intermediary.Issue issue: Issue object
@@ -500,30 +512,25 @@ def _get_preferred_issue_types(config, issue):
     #     'bug': 'Bug',
     #     'enhancement': 'Story'
     #   }
-    type_list = []
-
     cmap = config["sync2jira"].get("map", {})
     conf = cmap.get("github", {}).get(issue.upstream, {})
 
-    # We consider the issue_types mapping if it exists. If it does, exclude all other logic.
-    if "issue_types" in conf:
-        for tag, issue_type in conf["issue_types"].items():
-            if tag in issue.tags:
-                type_list.insert(0, issue_type)
-        type_list.sort()
-
-    # If issue_types was not provided, we consider the type option next.  If
-    # that is not set, fall back to the old behavior.
-    if not type_list:
-        if "type" in conf:
-            type_list.insert(0, conf["type"])
-        else:
-            if "RFE" in issue.title:
-                type_list.insert(0, "Story")
-            else:
-                type_list.insert(0, "Bug")
-    log.debug("Preferred issue type list: %s" % type_list)
-    return type_list
+    if issue_types := conf.get("issue_types"):
+        type_list = [v for k, v in issue_types.items() if k in issue.tags]
+        if type_list:
+            type_list.sort()
+            return type_list
+
+    if issue_type := conf.get("type"):
+        return [issue_type]
+
+    if issue.issue_type:
+        return [issue.issue_type]
+
+    if "RFE" in issue.title:
+        return ["Story"]
+
+    return ["Bug"]
 
 
 def _create_jira_issue(client, issue, config):
@@ -625,9 +632,8 @@ def _create_jira_issue(client, issue, config):
         )
         client.add_comment(downstream, comment)
     if len(preferred_types) > 1:
-        comment = "Some labels look like issue types but were not considered:" + str(
-            {preferred_types[1:]}
-        )
+        comment = "Some labels look like issue types but were not considered:  "
+        comment += str(preferred_types[1:])
         client.add_comment(downstream, comment)
 
     remote_link = dict(url=issue.url, title=remote_link_title)
diff --git a/sync2jira/intermediary.py b/sync2jira/intermediary.py
index 980dc48..9e0d3b8 100644
--- a/sync2jira/intermediary.py
+++ b/sync2jira/intermediary.py
@@ -38,9 +38,10 @@ class Issue(object):
         reporter,
         assignee,
         status,
-        id,
+        id_,
         storypoints,
         upstream_id,
+        issue_type,
         downstream=None,
     ):
         self.source = source
@@ -66,8 +67,9 @@ class Issue(object):
         self.reporter = reporter
         self.assignee = assignee
         self.status = status
-        self.id = str(id)
+        self.id = str(id_)
         self.upstream_id = upstream_id
+        self.issue_type = issue_type
         if not downstream:
             self.downstream = config["sync2jira"]["map"][self.source][upstream]
         else:
@@ -84,20 +86,9 @@ class Issue(object):
 
     @classmethod
     def from_github(cls, upstream, issue, config):
-        """Helper function to create intermediary object."""
+        """Helper function to create an intermediary Issue object."""
         upstream_source = "github"
-        comments = []
-        for comment in issue["comments"]:
-            comments.append(
-                {
-                    "author": comment["author"],
-                    "name": comment["name"],
-                    "body": trim_string(comment["body"]),
-                    "id": comment["id"],
-                    "date_created": comment["date_created"],
-                    "changed": None,
-                }
-            )
+        comments = reformat_github_comments(issue)
 
         # Reformat the state field
         if issue["state"]:
@@ -106,6 +97,11 @@ class Issue(object):
             elif issue["state"] == "closed":
                 issue["state"] = "Closed"
 
+        # Get the issue type if any
+        issue_type = issue.get("type")
+        if isinstance(issue_type, dict):
+            issue_type = issue_type.get("name")
+
         # Perform any mapping
         mapping = config["sync2jira"]["map"][upstream_source][upstream].get(
             "mapping", []
@@ -129,9 +125,10 @@ class Issue(object):
             reporter=issue["user"],
             assignee=issue["assignees"],
             status=issue["state"],
-            id=issue["id"],
+            id_=issue["id"],
             storypoints=issue.get("storypoints"),
             upstream_id=issue["number"],
+            issue_type=issue_type,
         )
 
     def __repr__(self):
@@ -155,7 +152,7 @@ class PR(object):
         reporter,
         assignee,
         status,
-        id,
+        id_,
         suffix,
         match,
         downstream=None,
@@ -188,7 +185,7 @@ class PR(object):
         self.reporter = reporter
         self.assignee = assignee
         self.status = status
-        self.id = str(id)
+        self.id = str(id_)
         self.suffix = suffix
         self.match = match
         # self.upstream_id = upstream_id
@@ -205,23 +202,12 @@ class PR(object):
 
     @classmethod
     def from_github(cls, upstream, pr, suffix, config):
-        """Helper function to create intermediary object."""
+        """Helper function to create an intermediary PR object."""
         # Set our upstream source
         upstream_source = "github"
 
         # Format our comments
-        comments = []
-        for comment in pr["comments"]:
-            comments.append(
-                {
-                    "author": comment["author"],
-                    "name": comment["name"],
-                    "body": trim_string(comment["body"]),
-                    "id": comment["id"],
-                    "date_created": comment["date_created"],
-                    "changed": None,
-                }
-            )
+        comments = reformat_github_comments(pr)
 
         # Build our URL
         url = pr["html_url"]
@@ -256,13 +242,27 @@ class PR(object):
             assignee=pr["assignee"],
             # GitHub PRs do not have status
             status=None,
-            id=pr["number"],
+            id_=pr["number"],
             # upstream_id=issue['number'],
             suffix=suffix,
             match=match,
         )
 
 
+def reformat_github_comments(issue):
+    return [
+        {
+            "author": comment["author"],
+            "name": comment["name"],
+            "body": trim_string(comment["body"]),
+            "id": comment["id"],
+            "date_created": comment["date_created"],
+            "changed": None,
+        }
+        for comment in issue["comments"]
+    ]
+
+
 def map_fixVersion(mapping, issue):
     """
     Helper function to perform any fixVersion mapping.
@@ -315,7 +315,7 @@ def matcher(content: Optional[str], comments: list[dict[str, str]]) -> str:
 
 def trim_string(content):
     """
-    Helper function to trim a string to ensure it is not over 50000 char
+    Helper function to trim a string to ensure it is not over 50,000 char
     Ref: https://github.com/release-engineering/Sync2Jira/issues/123
 
     :param String content: Comment content
diff --git a/sync2jira/main.py b/sync2jira/main.py
index 195e9f3..86d7dd7 100644
--- a/sync2jira/main.py
+++ b/sync2jira/main.py
@@ -311,8 +311,10 @@ def handle_msg(body, suffix, config):
         # is actually for a PR
         if "pull_request" in body["issue"]:
             if body["action"] == "deleted":
-                # FIXME:  What _should_ we be doing in this case?  Calling pr_handlers[]()??
-                log.info("Not handling PR 'action' == 'deleted'")
+                # I think this gets triggered when someone deletes a comment
+                # from a PR.  Since we don't capture PR comments (only Issue
+                # comments), we don't need to react if one is deleted.
+                log.debug("Not handling PR 'action' == 'deleted'")
                 return
             # Handle this PR update as though it were an Issue, if that's
             # acceptable to the configuration.
diff --git a/sync2jira/upstream_issue.py b/sync2jira/upstream_issue.py
index abd69e2..56007e3 100644
--- a/sync2jira/upstream_issue.py
+++ b/sync2jira/upstream_issue.py
@@ -167,7 +167,7 @@ def handle_github_message(body, config, is_pr=False):
                 )
                 return None
 
-    if is_pr and "closed_at" not in issue:
+    if is_pr and not issue.get("closed_at"):
         log.debug(
             "%r is a pull request.  Ignoring.", issue.get("html_url", "<missing URL>")
         )


Test patch:
diff --git a/tests/test_downstream_issue.py b/tests/test_downstream_issue.py
index 22f4ce2..3224f38 100644
--- a/tests/test_downstream_issue.py
+++ b/tests/test_downstream_issue.py
@@ -1,4 +1,5 @@
 from datetime import datetime, timezone
+from typing import Any, Optional
 import unittest
 import unittest.mock as mock
 from unittest.mock import MagicMock
@@ -76,6 +77,7 @@ class TestDownstreamIssue(unittest.TestCase):
         self.mock_issue.id = "1234"
         self.mock_issue.storypoints = 2
         self.mock_issue.priority = "P1"
+        self.mock_issue.issue_type = None
 
         # Mock issue updates
         self.mock_updates = [
@@ -399,15 +401,11 @@ class TestDownstreamIssue(unittest.TestCase):
         mock_client.assign_issue.assert_not_called()
         mock_client.search_assignable_users_for_issues.assert_not_called()
 
-    @mock.patch(PATH + "_update_jira_issue")
-    @mock.patch(PATH + "attach_link")
-    @mock.patch("jira.client.JIRA")
-    def test_create_jira_issue(
-        self, mock_client, mock_attach_link, mock_update_jira_issue
+    def common_test_create_jira_issue(
+        self, mock_attach_link, mock_client, mock_update_jira_issue
     ):
-        """
-        Tests '_create_jira_issue' function
-        """
+        """Common code for testing _create_jira_issue"""
+
         # Set up return values
         mock_client.create_issue.return_value = self.mock_downstream
         mock_client.fields.return_value = [
@@ -426,7 +424,11 @@ class TestDownstreamIssue(unittest.TestCase):
             issuetype={"name": "Bug"},
             project={"key": "mock_project"},
             somecustumfield="somecustumvalue",
-            description="[1234] Upstream Reporter: mock_user\nUpstream issue status: Open\nUpstream description: {quote}mock_content{quote}",
+            description=(
+                "[1234] Upstream Reporter: mock_user\n"
+                "Upstream issue status: Open\n"
+                "Upstream description: {quote}mock_content{quote}"
+            ),
             summary="mock_title",
         )
         mock_attach_link.assert_called_with(
@@ -445,6 +447,20 @@ class TestDownstreamIssue(unittest.TestCase):
             {"customfield_3": {"value": "EXD-Project", "child": {"value": "EXD-Value"}}}
         )
         self.assertEqual(response, self.mock_downstream)
+
+    @mock.patch(PATH + "_update_jira_issue")
+    @mock.patch(PATH + "attach_link")
+    @mock.patch("jira.client.JIRA")
+    def test_create_jira_issue(
+        self, mock_client, mock_attach_link, mock_update_jira_issue
+    ):
+        """
+        Tests '_create_jira_issue' function normal success case
+        """
+        self.common_test_create_jira_issue(
+            mock_attach_link, mock_client, mock_update_jira_issue
+        )
+
         mock_client.add_comment.assert_not_called()
 
     @mock.patch(PATH + "_update_jira_issue")
@@ -457,43 +473,12 @@ class TestDownstreamIssue(unittest.TestCase):
         Tests '_create_jira_issue' function when we fail while updating the epic link
         """
         # Set up return values
-        mock_client.create_issue.return_value = self.mock_downstream
-        mock_client.fields.return_value = [
-            {"name": "Epic Link", "id": "customfield_1"},
-            {"name": "QA Contact", "id": "customfield_2"},
-            {"name": "EXD-Service", "id": "customfield_3"},
-        ]
         self.mock_downstream.update.side_effect = [JIRAError, "success", "success"]
 
-        # Call the function
-        response = d._create_jira_issue(
-            client=mock_client, issue=self.mock_issue, config=self.mock_config
+        self.common_test_create_jira_issue(
+            mock_attach_link, mock_client, mock_update_jira_issue
         )
 
-        # Assert everything was called correctly
-        mock_client.create_issue.assert_called_with(
-            issuetype={"name": "Bug"},
-            project={"key": "mock_project"},
-            somecustumfield="somecustumvalue",
-            description="[1234] Upstream Reporter: mock_user\nUpstream issue status: Open\nUpstream description: {quote}mock_content{quote}",
-            summary="mock_title",
-        )
-        mock_attach_link.assert_called_with(
-            mock_client,
-            self.mock_downstream,
-            {"url": "mock_url", "title": "Upstream issue"},
-        )
-        mock_update_jira_issue.assert_called_with(
-            self.mock_downstream, self.mock_issue, mock_client, self.mock_config
-        )
-        self.mock_downstream.update.assert_any_call({"customfield_1": "DUMMY-1234"})
-        self.mock_downstream.update.assert_any_call(
-            {"customfield_2": "dummy@dummy.com"}
-        )
-        self.mock_downstream.update.assert_any_call(
-            {"customfield_3": {"value": "EXD-Project", "child": {"value": "EXD-Value"}}}
-        )
-        self.assertEqual(response, self.mock_downstream)
         mock_client.add_comment.assert_called_with(
             self.mock_downstream, f"Error adding Epic-Link: DUMMY-1234"
         )
@@ -509,43 +494,12 @@ class TestDownstreamIssue(unittest.TestCase):
         EXD-Service field
         """
         # Set up return values
-        mock_client.create_issue.return_value = self.mock_downstream
-        mock_client.fields.return_value = [
-            {"name": "Epic Link", "id": "customfield_1"},
-            {"name": "QA Contact", "id": "customfield_2"},
-            {"name": "EXD-Service", "id": "customfield_3"},
-        ]
         self.mock_downstream.update.side_effect = ["success", "success", JIRAError]
 
-        # Call the function
-        response = d._create_jira_issue(
-            client=mock_client, issue=self.mock_issue, config=self.mock_config
+        self.common_test_create_jira_issue(
+            mock_attach_link, mock_client, mock_update_jira_issue
         )
 
-        # Assert everything was called correctly
-        mock_client.create_issue.assert_called_with(
-            issuetype={"name": "Bug"},
-            project={"key": "mock_project"},
-            somecustumfield="somecustumvalue",
-            description="[1234] Upstream Reporter: mock_user\nUpstream issue status: Open\nUpstream description: {quote}mock_content{quote}",
-            summary="mock_title",
-        )
-        mock_attach_link.assert_called_with(
-            mock_client,
-            self.mock_downstream,
-            {"url": "mock_url", "title": "Upstream issue"},
-        )
-        mock_update_jira_issue.assert_called_with(
-            self.mock_downstream, self.mock_issue, mock_client, self.mock_config
-        )
-        self.mock_downstream.update.assert_any_call({"customfield_1": "DUMMY-1234"})
-        self.mock_downstream.update.assert_any_call(
-            {"customfield_2": "dummy@dummy.com"}
-        )
-        self.mock_downstream.update.assert_any_call(
-            {"customfield_3": {"value": "EXD-Project", "child": {"value": "EXD-Value"}}}
-        )
-        self.assertEqual(response, self.mock_downstream)
         mock_client.add_comment.assert_called_with(
             self.mock_downstream,
             f"Error adding EXD-Service field.\n"
@@ -553,6 +507,33 @@ class TestDownstreamIssue(unittest.TestCase):
             f"Value: {self.mock_issue.downstream['EXD-Service']['value']}",
         )
 
+    @mock.patch(PATH + "_update_jira_issue")
+    @mock.patch(PATH + "attach_link")
+    @mock.patch(PATH + "_get_preferred_issue_types")
+    @mock.patch("jira.client.JIRA")
+    def test_create_jira_issue_multiple_types(
+        self,
+        mock_client,
+        mock_get_preferred_issue_types,
+        mock_attach_link,
+        mock_update_jira_issue,
+    ):
+        """
+        Tests '_create_jira_issue' function when multiple possible issue types are found
+        """
+        # Set up return values
+        issue_types = ["Bug", "Feature", "Outcome", "Story"]
+        mock_get_preferred_issue_types.return_value = issue_types
+
+        self.common_test_create_jira_issue(
+            mock_attach_link, mock_client, mock_update_jira_issue
+        )
+
+        mock_client.add_comment.assert_called_with(
+            self.mock_downstream,
+            f"Some labels look like issue types but were not considered:  {issue_types[1:]}",
+        )
+
     @mock.patch(PATH + "_update_jira_issue")
     @mock.patch(PATH + "attach_link")
     @mock.patch("jira.client.JIRA")
@@ -591,6 +572,83 @@ class TestDownstreamIssue(unittest.TestCase):
         self.assertEqual(response, self.mock_downstream)
         mock_client.add_comment.assert_not_called()
 
+    def test_get_preferred_issue_types(self):
+        """
+        Tests '_get_preferred_issue_types' function
+
+        Scenarios:
+         - configuration has type mappings
+            - first mapping matches
+            - second mapping matches
+            - multiple mappings match
+            - no mapping matches
+         - configuration has a default type
+         - upstream issue has a type
+         - "RFE" in issue title
+         - None of the above.
+        """
+        conf = {
+            "issue_types": {
+                "tag1": "mapped_type_C",  # In reverse-sorted order to test sorting
+                "tag2": "mapped_type_B",
+                "tag3": "mapped_type_A",
+            },
+            "type": "S2J_type",
+        }
+        self.mock_config["sync2jira"]["map"] = {
+            "github": {self.mock_issue.upstream: conf}
+        }
+        self.mock_issue.issue_type = "GH_type"
+        self.mock_issue.tags = ["tag1"]
+        self.mock_issue.title = "RFE: Mock Issue Title"
+
+        def update_state(
+            issue_field: Optional[dict[str, Any]] = None, config: Optional[str] = None
+        ):
+            if issue_field:
+                for k, v in issue_field.items():
+                    self.mock_issue.__setattr__(k, v)
+            if config:
+                del conf[config]
+
+        # List of scenarios:  each entry includes a callable which modifies
+        # either the mock issue or the mock configuration prior to invoking the
+        # CUT and the resulting value which is expected to be returned; the
+        # test iterates over the list, calling each setup function, calling the
+        # CUT, and then comparing the result to the expected value.
+        scenarios = (
+            # The issue label matches the first in the configured type map.
+            (lambda: None, ["mapped_type_C"]),
+            # The issue label matches the second in the configured type map.
+            (
+                lambda: update_state(issue_field={"tags": ["tag2"]}),
+                ["mapped_type_B"],
+            ),
+            # The issue label has multiple matches in the configured type map.
+            (
+                lambda: update_state(issue_field={"tags": ["tag2", "tag1"]}),
+                ["mapped_type_B", "mapped_type_C"],
+            ),
+            # The issue label has no matches in the configured type map.
+            (lambda: update_state(issue_field={"tags": ["fred"]}), ["S2J_type"]),
+            # There is no type map, but the configuration specifies a default type.
+            (lambda: update_state(config="issue_types"), ["S2J_type"]),
+            # No type in the configuration, but the upstream issue has a type.
+            (lambda: update_state(config="type"), ["GH_type"]),
+            # No type from config or upstream, but there is "RFE" in issue title.
+            (lambda: update_state(issue_field={"issue_type": None}), ["Story"]),
+            # Default fallback
+            (
+                lambda: update_state(issue_field={"title": "Plain Issue Title"}),
+                ["Bug"],
+            ),
+        )
+
+        for scenario, (setup_action, expected) in enumerate(scenarios):
+            setup_action()
+            actual = d._get_preferred_issue_types(self.mock_config, self.mock_issue)
+            self.assertEqual(actual, expected, f"In scenario {scenario}")
+
     @mock.patch(PATH + "get_jira_client")
     @mock.patch(PATH + "_get_existing_jira_issue")
     @mock.patch(PATH + "_update_jira_issue")
diff --git a/tests/test_intermediary.py b/tests/test_intermediary.py
index 942f52c..3c84513 100644
--- a/tests/test_intermediary.py
+++ b/tests/test_intermediary.py
@@ -41,6 +41,7 @@ class TestIntermediary(unittest.TestCase):
             "date_created": "mock_date",
             "number": "1",
             "storypoints": "mock_storypoints",
+            "type": None,
         }
 
         self.mock_github_pr = {
@@ -107,6 +108,7 @@ class TestIntermediary(unittest.TestCase):
         self.assertEqual(response.status, "Open")
         self.assertEqual(response.downstream, {"mock_downstream": "mock_key"})
         self.assertEqual(response.storypoints, "mock_storypoints")
+        self.assertEqual(response.issue_type, None)
 
     def test_from_github_open_without_priority(self):
         """
@@ -170,6 +172,30 @@ class TestIntermediary(unittest.TestCase):
         self.assertEqual(response.downstream, {"mock_downstream": "mock_key"})
         self.assertEqual(response.storypoints, "mock_storypoints")
 
+    def test_from_github_with_type(self):
+        """
+        This tests the 'from_github' function under the Issue class with
+        various values in the 'type' field
+        """
+        for issue_type, expected in (
+            (None, None),
+            ({}, None),
+            ({"name": None}, None),
+            ({"name": "issue_type_name"}, "issue_type_name"),
+            ({"fred": 1}, None),
+        ):
+            # Set up return values
+            self.mock_github_issue["type"] = issue_type
+
+            # Call the function
+            response = i.Issue.from_github(
+                upstream="github", issue=self.mock_github_issue, config=self.mock_config
+            )
+
+            # Assert that we made the calls correctly
+            self.checkResponseFields(response)
+            self.assertEqual(expected, response.issue_type)
+
     def test_mapping_github(self):
         """
         This tests the mapping feature from GitHub