Merge pull request #441 from Bala-Sakabattula/GDAP-issue

gdap issue fix

Author: Bala-Sakabattula

sync2jira/downstream_issue.py

@@ -23,7 +23,7 @@
 import operator
 import os
 import re
-from typing import Any, Optional, Union
+from typing import Any, Dict, Optional, Union
 import unicodedata
 
 from dotenv import load_dotenv
@@ -568,38 +568,46 @@ def _upgrade_jira_issue(client, downstream, issue, config):
     attach_link(client, downstream, remote_link)
 
 
-def match_user(emails: list[str], client: jira.client.JIRA) -> Optional[str]:
-    """Match an upstream user to an assignable downstream user and return the
-    downstream username; return None on failure.
+def match_user(emails: list[str], client: jira.client.JIRA) -> Optional[Dict[str, str]]:
+    """Match an upstream user to an assignable downstream Jira user.
+
+    Returns a dict with ``name`` (display name) and ``accountId`` for Jira Cloud,
+    or None on failure.
     """
 
     for email in emails:
         # Get a list from Jira of users that match the supplied email address.
-        users = client.search_users(user=email)
+        # Use query= for Jira Cloud (GDPR strict mode rejects the username param).
+        users = client.search_users(query=email)
 
         if not users:
             continue
 
         if len(users) == 1:
-            # TODO:  We should probably return the ID here, instead.
-            return users[0].name
+            u = users[0]
+            return {
+                "name": getattr(u, "displayName", None) or "<name-not-available>",
+                "accountId": getattr(u, "accountId", None),
+            }
 
         limit = 5
         log.warning(
             "Found %d Jira users for %r:  %s%s",
             len(users),
             email,
-            ", ".join(u.name for u in users[0:limit]),
+            ", ".join(
+                getattr(u, "displayName", "<name-not-available>")
+                for u in users[0:limit]
+            ),
             "..." if len(users) > limit else "",
         )
         for user in users:
-            # This condition _should_ be true for *all* entries returned, in
-            # which case we'll just return the first entry; however it appears
-            # that sometimes Jira returns _all_ assignable users, so do our own
-            # filtering.
-            if user.emailAddress == email:
-                log.info("Found matching user: %r", user.name)
-                return user.name
+            # Filter by email when present (can be omitted in Cloud when hidden).
+            if getattr(user, "emailAddress", None) == email:
+                name = getattr(user, "displayName", None) or "<no-name-available>"
+                aid = getattr(user, "accountId", None)
+                log.info("Found matching user: %r", name)
+                return {"name": name, "accountId": aid}
         else:
             log.warning("Found no Jira user which matches %r", email)
 
@@ -637,11 +645,16 @@ def assign_user(
             continue
 
         # Try to match the upstream assignee's emails to a Jira user
-        match_name = match_user(emails, client)
-        if match_name:
-            # Assign the downstream issue to the matched user
-            downstream.update({"assignee": {"name": match_name}})
-            log.info("Assigned %s to %r", downstream.key, match_name)
+        matched = match_user(emails, client)
+        if matched and matched["accountId"]:
+            # Jira Cloud assigns by accountId
+            downstream.update({"assignee": {"accountId": matched["accountId"]}})
+            log.info(
+                "Assigned %s to %r,%r",
+                downstream.key,
+                matched["name"],
+                matched["accountId"],
+            )
             return
 
     if issue.assignee:

sync2jira/downstream_pr.py

@@ -41,12 +41,12 @@ def format_comment(pr, pr_suffix, client):
     :return: Formatted comment
     :rtype: String
     """
-    # Find the pr.reporters JIRA username
-    ret = client.search_users(pr.reporter)
+    # Find the pr.reporter's Jira user (use query= for Jira Cloud GDPR strict mode).
+    ret = client.search_users(query=pr.reporter)
     # Loop through ret till we find a match
     for user in ret:
-        if user.displayName == pr.reporter:
-            reporter = f"[~{user.key}]"
+        if getattr(user, "displayName", "") == pr.reporter:
+            reporter = f"[~accountId:{user.accountId}]"
             break
     else:
         reporter = pr.reporter

tests/test_downstream_issue.py

@@ -516,6 +516,7 @@ def test_assign_user(self, mock_client, mock_rover_lookup):
         mock_user = MagicMock()
         mock_user.displayName = "mock_assignee"
         mock_user.key = "mock_user_key"
+        mock_user.accountId = "mock-account-id-assign"
         mock_client.search_users.return_value = [mock_user]
         mock_client.assign_issue.return_value = True
         mock_rover_lookup.return_value = ["mock_user@redhat.com"]
@@ -525,11 +526,11 @@ def test_assign_user(self, mock_client, mock_rover_lookup):
             issue=self.mock_issue, downstream=self.mock_downstream, client=mock_client
         )
 
-        # Assert that all calls mocked were called properly
+        # Assert that all calls mocked were called properly (Jira Cloud: accountId)
         self.mock_downstream.update.assert_called_with(
-            {"assignee": {"name": mock_user.name}}
+            {"assignee": {"accountId": mock_user.accountId}}
         )
-        mock_client.search_users.assert_called_with(user="mock_user@redhat.com")
+        mock_client.search_users.assert_called_with(query="mock_user@redhat.com")
 
     @mock.patch("Rover_Lookup.github_username_to_emails")
     @mock.patch("jira.client.JIRA")
@@ -542,6 +543,7 @@ def test_assign_user_diacritics(self, mock_client, mock_rover_lookup):
         mock_user = MagicMock()
         mock_user.displayName = "mock_assignee"
         mock_user.key = "mock_user_key"
+        mock_user.accountId = "mock-account-id-diacritics"
         mock_client.search_users.return_value = [mock_user]
         mock_client.assign_issue.return_value = True
         mock_rover_lookup.return_value = ["mock_user@redhat.com"]
@@ -555,9 +557,9 @@ def test_assign_user_diacritics(self, mock_client, mock_rover_lookup):
 
         # Assert that all calls mocked were called properly
         self.mock_downstream.update.assert_called_with(
-            {"assignee": {"name": mock_user.name}}
+            {"assignee": {"accountId": mock_user.accountId}}
         )
-        mock_client.search_users.assert_called_with(user="mock_user@redhat.com")
+        mock_client.search_users.assert_called_with(query="mock_user@redhat.com")
 
     @mock.patch("Rover_Lookup.github_username_to_emails")
     @mock.patch("jira.client.JIRA")
@@ -572,11 +574,13 @@ def test_assign_user_multiple(self, mock_client, mock_rover_lookup):
         mock_user.name = "mock_assignee_name"
         mock_user.emailAddress = "mock_user@redhat.com"
         mock_user.key = "mock_user_key"
+        mock_user.accountId = "mock-account-id-multi"
         mock_user2 = MagicMock()
         mock_user2.displayName = "mock_assignee2"
         mock_user2.name = "mock_assignee2_name"
         mock_user2.emailAddress = "wrong_mock_user@redhat.com"
         mock_user2.key = "mock_user2_key"
+        mock_user2.accountId = "mock-account-id-2"
         mock_client.search_users.return_value = [
             mock_user,
             mock_user2,
@@ -608,9 +612,9 @@ def test_assign_user_multiple(self, mock_client, mock_rover_lookup):
 
         # Assert that all calls mocked were called properly
         self.mock_downstream.update.assert_called_with(
-            {"assignee": {"name": mock_user.name}}
+            {"assignee": {"accountId": mock_user.accountId}}
         )
-        mock_client.search_users.assert_called_with(user="mock_user@redhat.com")
+        mock_client.search_users.assert_called_with(query="mock_user@redhat.com")
 
     @mock.patch("Rover_Lookup.github_username_to_emails")
     @mock.patch("jira.client.JIRA")
@@ -658,7 +662,7 @@ def test_assign_user_with_owner_no_match(self, mock_client, mock_rover_lookup):
 
         # Assert that all calls mocked were called properly
         mock_client.assign_issue.assert_called_with(1234, "mock_owner")
-        mock_client.search_users.assert_called_with(user="no_match@redhat.com")
+        mock_client.search_users.assert_called_with(query="no_match@redhat.com")
 
     @mock.patch("Rover_Lookup.github_username_to_emails")
     @mock.patch("jira.client.JIRA")

tests/test_downstream_pr.py

@@ -50,6 +50,7 @@ def setUp(self):
         mock_user = MagicMock()
         mock_user.displayName = "mock_reporter"
         mock_user.key = "mock_key"
+        mock_user.accountId = "mock-atlassian-account-id"
         self.mock_client.search_users.return_value = [mock_user]
         self.mock_client.search_issues.return_value = ["mock_existing"]
 
@@ -331,9 +332,10 @@ def test_format_comment_open(self):
         response = d.format_comment(self.mock_pr, "open", self.mock_client)
 
         # Assert Everything was called correctly
+        self.mock_client.search_users.assert_called_with(query="mock_reporter")
         self.assertEqual(
             response,
-            "[~mock_key] mentioned this issue in merge request [mock_title| mock_url].",
+            "[~accountId:mock-atlassian-account-id] mentioned this issue in merge request [mock_title| mock_url].",
         )
 
     def test_format_comment_open_no_user_found(self):
@@ -347,6 +349,7 @@ def test_format_comment_open_no_user_found(self):
         response = d.format_comment(self.mock_pr, "open", self.mock_client)
 
         # Assert Everything was called correctly
+        self.mock_client.search_users.assert_called_with(query="mock_reporter")
         self.assertEqual(
             response,
             "mock_reporter mentioned this issue in merge request [mock_title| mock_url].",