If create_pr_issue is set, search before creating (#413)

* If create_pr_issue is set, search before creating

* Take two

Author: webbnh

sync2jira/downstream_issue.py

@@ -294,7 +294,7 @@ def get_jira_client(issue, config):
     return client
 
 
-def _get_existing_jira_issue(client, issue, config):
+def get_existing_jira_issue(client, issue, config):
     """
     Get a jira issue by the linked remote issue.
 
@@ -1414,7 +1414,7 @@ def update_jira(client, config, issue):
     # First, check to see if we have a matching issue using the new method.
     # If we do, then bail out.  No sync needed.
     log.info("Looking for matching downstream issue via new method.")
-    existing = _get_existing_jira_issue(client, issue, config)
+    existing = get_existing_jira_issue(client, issue, config)
     if existing:
         # If we found an existing JIRA issue already
         log.info("Found existing, matching downstream %r.", existing.key)

sync2jira/downstream_pr.py

@@ -225,8 +225,11 @@ def update_jira(client, config, pr):
     if not pr.jira_key:
         create_pr_issue = pr.downstream.get("create_pr_issue", False)
         if create_pr_issue:
-            # Convert PR to Issue-like object for creation
-            _create_jira_issue_from_pr(client, pr, config)
+            if existing := d_issue.get_existing_jira_issue(client, pr, config):
+                log.info(f"Found existing JIRA issue {existing.key} for PR {pr.url}")
+                update_jira_issue(existing, pr, client)
+            else:
+                _create_jira_issue_from_pr(client, pr, config)
         else:
             log.info("No JIRA key found in PR, skipping.")
         return

tests/test_downstream_issue.py

@@ -191,7 +191,7 @@ class MockIssue(object):
     @mock.patch("jira.client.JIRA")
     def test_get_existing_newstyle(self, mock_client, mock_get_query, mock_filter):
         """
-        This tests '_get_existing_jira_issue' function.
+        This tests 'get_existing_jira_issue' function.
         """
         mock_issue_1: JIssue = MagicMock(spec=JIssue, name="mock_issue_1")
         mock_issue_1.key = "MOCK-1"
@@ -248,7 +248,7 @@ def test_get_existing_newstyle(self, mock_client, mock_get_query, mock_filter):
             mock_get_query.return_value = x["jira_results"]
             mock_client.search_issues.return_value = x["search_issues"]
             mock_filter.return_value = x["filter_results"]
-            result = d._get_existing_jira_issue(
+            result = d.get_existing_jira_issue(
                 client=mock_client, issue=self.mock_issue, config=self.mock_config
             )
             self.assertEqual(result, x["expected"])
@@ -893,7 +893,7 @@ def update_state(
             self.assertEqual(actual, expected, f"In scenario {scenario}")
 
     @mock.patch(PATH + "get_jira_client")
-    @mock.patch(PATH + "_get_existing_jira_issue")
+    @mock.patch(PATH + "get_existing_jira_issue")
     @mock.patch(PATH + "_update_jira_issue")
     @mock.patch(PATH + "_create_jira_issue")
     @mock.patch("jira.client.JIRA")
@@ -930,7 +930,7 @@ def test_sync_with_jira_matching(
         mock_existing_jira_issue_legacy.assert_not_called()
 
     @mock.patch(PATH + "get_jira_client")
-    @mock.patch(PATH + "_get_existing_jira_issue")
+    @mock.patch(PATH + "get_existing_jira_issue")
     @mock.patch(PATH + "_update_jira_issue")
     @mock.patch(PATH + "_create_jira_issue")
     @mock.patch("jira.client.JIRA")
@@ -965,7 +965,7 @@ def test_sync_with_jira_down(
         mock_existing_jira_issue_legacy.assert_not_called()
 
     @mock.patch(PATH + "get_jira_client")
-    @mock.patch(PATH + "_get_existing_jira_issue")
+    @mock.patch(PATH + "get_existing_jira_issue")
     @mock.patch(PATH + "_update_jira_issue")
     @mock.patch(PATH + "_create_jira_issue")
     @mock.patch("jira.client.JIRA")

tests/test_downstream_pr.py

@@ -411,51 +411,189 @@ def test_sync_with_jira_create_pr_issue_disabled(
         # Assert everything was called correctly
         mock_update_jira.assert_not_called()
 
+    @mock.patch(PATH + "d_issue")
+    def test_update_jira_service_unavailable(self, mock_d_issue):
+        """Test 'update_jira' when in development mode and the Jira service is unavailable."""
+
+        # Set up return values
+        self.mock_config["sync2jira"]["develop"] = False
+        mock_d_issue.check_jira_status.return_value = False
+
+        # Call the function
+        with self.assertRaises(RuntimeError):
+            d.update_jira(self.mock_client, self.mock_config, self.mock_pr)
+
     @mock.patch(PATH + "_create_jira_issue_from_pr")
+    @mock.patch(PATH + "update_jira_issue")
+    @mock.patch(PATH + "matcher")
     @mock.patch(PATH + "d_issue")
-    def test_update_jira_create_pr_issue_enabled(
-        self, mock_d_issue, mock_create_jira_issue_from_pr
+    def test_update_jira_with_no_key_create_enabled_no_issue(
+        self,
+        mock_d_issue,
+        mock_matcher,
+        mock_update_jira_issue,
+        mock_create_jira_issue_from_pr,
     ):
-        """
-        Test 'update_jira' when create_pr_issue is enabled and no JIRA key is found.
+        """Test 'update_jira' when no JIRA key is found, issue creation is
+        enabled, and the issue is not found.
         """
         # Set up return values
-        mock_client = MagicMock()
         mock_d_issue.check_jira_status.return_value = True
-        self.mock_pr.jira_key = None
+        mock_matcher.return_value = self.mock_pr.jira_key = None
         self.mock_pr.downstream = {"create_pr_issue": True}
+        mock_d_issue.get_existing_jira_issue.return_value = None
 
         # Call the function
-        d.update_jira(mock_client, self.mock_config, self.mock_pr)
+        d.update_jira(self.mock_client, self.mock_config, self.mock_pr)
 
         # Assert everything was called correctly
-        mock_d_issue.check_jira_status.assert_called_with(mock_client)
-        mock_create_jira_issue_from_pr.assert_called_with(
-            mock_client, self.mock_pr, self.mock_config
-        )
-        mock_client.search_issues.assert_not_called()
+        mock_update_jira_issue.assert_not_called()
+        mock_create_jira_issue_from_pr.assert_called()
 
     @mock.patch(PATH + "_create_jira_issue_from_pr")
+    @mock.patch(PATH + "update_jira_issue")
+    @mock.patch(PATH + "matcher")
     @mock.patch(PATH + "d_issue")
-    def test_update_jira_create_pr_issue_disabled(
-        self, mock_d_issue, mock_create_jira_issue_from_pr
+    def test_update_jira_with_no_key_create_enabled_issue_exists(
+        self,
+        mock_d_issue,
+        mock_matcher,
+        mock_update_jira_issue,
+        mock_create_jira_issue_from_pr,
     ):
-        """
-        Test 'update_jira' when create_pr_issue is disabled and no JIRA key is found.
+        """Test 'update_jira' when no JIRA key is found, issue creation is
+        enabled, and the issue exists.
         """
         # Set up return values
-        mock_client = MagicMock()
         mock_d_issue.check_jira_status.return_value = True
-        self.mock_pr.jira_key = None
+        mock_matcher.return_value = self.mock_pr.jira_key = None
+        self.mock_pr.downstream = {"create_pr_issue": True}
+        mock_d_issue.get_existing_jira_issue.return_value = self.mock_existing
+
+        # Call the function
+        d.update_jira(self.mock_client, self.mock_config, self.mock_pr)
+
+        # Assert everything was called correctly
+        mock_update_jira_issue.assert_called()
+        mock_create_jira_issue_from_pr.assert_not_called()
+
+    @mock.patch(PATH + "_create_jira_issue_from_pr")
+    @mock.patch(PATH + "update_jira_issue")
+    @mock.patch(PATH + "matcher")
+    @mock.patch(PATH + "d_issue")
+    def test_update_jira_with_no_key_create_disabled(
+        self,
+        mock_d_issue,
+        mock_matcher,
+        mock_update_jira_issue,
+        mock_create_jira_issue_from_pr,
+    ):
+        """Test 'update_jira' when no JIRA key is found and issue creation is disabled."""
+        # Set up return values
+        mock_d_issue.check_jira_status.return_value = True
+        mock_matcher.return_value = self.mock_pr.jira_key = None
         self.mock_pr.downstream = {"create_pr_issue": False}
 
         # Call the function
-        d.update_jira(mock_client, self.mock_config, self.mock_pr)
+        d.update_jira(self.mock_client, self.mock_config, self.mock_pr)
+
+        # Assert everything was (not) called correctly
+        mock_update_jira_issue.assert_not_called()
+        mock_create_jira_issue_from_pr.assert_not_called()
+
+    @mock.patch(PATH + "_create_jira_issue_from_pr")
+    @mock.patch(PATH + "update_jira_issue")
+    @mock.patch(PATH + "matcher")
+    @mock.patch(PATH + "d_issue")
+    def test_update_jira_with_no_key_create_unspecified(
+        self,
+        mock_d_issue,
+        mock_matcher,
+        mock_update_jira_issue,
+        mock_create_jira_issue_from_pr,
+    ):
+        """Test 'update_jira' when no JIRA key is found and issue creation is unspecified."""
+        # Set up return values
+        mock_d_issue.check_jira_status.return_value = True
+        mock_matcher.return_value = self.mock_pr.jira_key = None
+
+        # Call the function
+        d.update_jira(self.mock_client, self.mock_config, self.mock_pr)
+
+        # Assert everything was (not) called correctly
+        mock_update_jira_issue.assert_not_called()
+        mock_create_jira_issue_from_pr.assert_not_called()
+
+    @mock.patch(PATH + "_create_jira_issue_from_pr")
+    @mock.patch(PATH + "update_jira_issue")
+    @mock.patch(PATH + "matcher")
+    @mock.patch(PATH + "d_issue")
+    def test_update_jira_with_key_found_and_unique_issue(
+        self,
+        mock_d_issue,
+        mock_matcher,
+        mock_update_jira_issue,
+        mock_create_jira_issue_from_pr,
+    ):
+        """Test 'update_jira' when JIRA key is found and the issue is found."""
+        # Set up return values
+        mock_d_issue.check_jira_status.return_value = True
+        mock_matcher.return_value = self.mock_pr.jira_key
+
+        # Call the function
+        d.update_jira(self.mock_client, self.mock_config, self.mock_pr)
 
         # Assert everything was called correctly
-        mock_d_issue.check_jira_status.assert_called_with(mock_client)
+        mock_update_jira_issue.assert_called()
+        mock_create_jira_issue_from_pr.assert_not_called()
+
+    @mock.patch(PATH + "_create_jira_issue_from_pr")
+    @mock.patch(PATH + "update_jira_issue")
+    @mock.patch(PATH + "matcher")
+    @mock.patch(PATH + "d_issue")
+    def test_update_jira_with_key_found_and_no_issue(
+        self,
+        mock_d_issue,
+        mock_matcher,
+        mock_update_jira_issue,
+        mock_create_jira_issue_from_pr,
+    ):
+        """Test 'update_jira' when JIRA key is found and the issue is not found."""
+        # Set up return values
+        mock_d_issue.check_jira_status.return_value = True
+        mock_matcher.return_value = self.mock_pr.jira_key
+        self.mock_client.search_issues.return_value = []
+
+        # Call the function
+        d.update_jira(self.mock_client, self.mock_config, self.mock_pr)
+
+        # Assert everything was (not) called correctly
+        mock_update_jira_issue.assert_not_called()
+        mock_create_jira_issue_from_pr.assert_not_called()
+
+    @mock.patch(PATH + "_create_jira_issue_from_pr")
+    @mock.patch(PATH + "update_jira_issue")
+    @mock.patch(PATH + "matcher")
+    @mock.patch(PATH + "d_issue")
+    def test_update_jira_with_key_found_and_multiple_issues(
+        self,
+        mock_d_issue,
+        mock_matcher,
+        mock_update_jira_issue,
+        mock_create_jira_issue_from_pr,
+    ):
+        """Test 'update_jira' when JIRA key is found and the search returns multiple issues."""
+        # Set up return values
+        mock_d_issue.check_jira_status.return_value = True
+        mock_matcher.return_value = self.mock_pr.jira_key
+        self.mock_client.search_issues.return_value = ["MOCK-123", "MOCK-456"]
+
+        # Call the function
+        d.update_jira(self.mock_client, self.mock_config, self.mock_pr)
+
+        # Assert everything was called correctly:  should exit without calling subroutines
+        mock_update_jira_issue.assert_not_called()
         mock_create_jira_issue_from_pr.assert_not_called()
-        mock_client.search_issues.assert_not_called()
 
     def _setup_pr_for_issue_creation(self, **overrides):
         """