fix: prioritize user name over key for Jira Server/DC assignee field (#298)

Fixes a bug where setting assignees in Jira Server/Data Center environments would fail due to
providing the user's key instead of name in the assignee field. Modified lookup functions to
prioritize returning the user's name for non-cloud environments to ensure compatibility with
Jira Server/DC API that expects the 'name' value in the assignee.name field.

Author: sooperset

src/mcp_atlassian/jira/issues.py

@@ -540,8 +540,9 @@ def create_issue(
             # Add assignee if provided
             if assignee:
                 try:
-                    account_id = self._get_account_id(assignee)
-                    self._add_assignee_to_fields(fields, account_id)
+                    # _get_account_id now returns the correct identifier (accountId for cloud, name for server)
+                    assignee_identifier = self._get_account_id(assignee)
+                    self._add_assignee_to_fields(fields, assignee_identifier)
                 except ValueError as e:
                     logger.warning(f"Could not assign issue: {str(e)}")
 
@@ -1287,8 +1288,9 @@ def batch_create_issues(
                 # Add assignee if provided
                 if assignee:
                     try:
-                        account_id = self._get_account_id(assignee)
-                        self._add_assignee_to_fields(fields, account_id)
+                        # _get_account_id now returns the correct identifier (accountId for cloud, name for server)
+                        assignee_identifier = self._get_account_id(assignee)
+                        self._add_assignee_to_fields(fields, assignee_identifier)
                     except ValueError as e:
                         logger.warning(f"Could not assign issue: {str(e)}")
 

src/mcp_atlassian/jira/users.py

@@ -140,21 +140,25 @@ def _lookup_user_directly(self, username: str) -> str | None:
                     or user.get("name", "").lower() == username.lower()
                     or user.get("emailAddress", "").lower() == username.lower()
                 ):
-                    # Jira Cloud uses accountId
-                    if "accountId" in user:
-                        return user["accountId"]
-                    # Jira Data Center/Server might use key
-                    elif "key" in user:
-                        logger.info(
-                            "Using 'key' instead of 'accountId' for Jira Data Center/Server"
-                        )
-                        return user["key"]
-                    # Last resort fallback to name
-                    elif "name" in user:
-                        logger.info(
-                            "Using 'name' instead of 'accountId' for Jira Data Center/Server"
-                        )
-                        return user["name"]
+                    # Prioritize based on Cloud vs Server/DC for assignee field compatibility
+                    if self.config.is_cloud:
+                        # Cloud requires accountId
+                        if "accountId" in user:
+                            return user["accountId"]
+                    else:
+                        # Server/DC requires 'name' for the assignee field { "name": ... }
+                        if "name" in user:
+                            logger.info(
+                                "Using 'name' for assignee field in Jira Data Center/Server"
+                            )
+                            return user["name"]
+                        # Fallback to key if name is somehow missing (less common)
+                        elif "key" in user:
+                            logger.info(
+                                "Using 'key' as fallback for assignee name in Jira Data Center/Server"
+                            )
+                            return user["key"]
+
             return None
         except Exception as e:
             logger.info(f"Error looking up user directly: {str(e)}")
@@ -195,21 +199,24 @@ def _lookup_user_by_permissions(self, username: str) -> str | None:
             if response.status_code == 200:
                 data = response.json()
                 for user in data.get("users", []):
-                    # Jira Cloud uses accountId
-                    if "accountId" in user:
-                        return user["accountId"]
-                    # Jira Data Center/Server might use key
-                    elif "key" in user:
-                        logger.info(
-                            "Using 'key' instead of 'accountId' for Jira Data Center/Server"
-                        )
-                        return user["key"]
-                    # Last resort fallback to name
-                    elif "name" in user:
-                        logger.info(
-                            "Using 'name' instead of 'accountId' for Jira Data Center/Server"
-                        )
-                        return user["name"]
+                    # Prioritize based on Cloud vs Server/DC for assignee field compatibility
+                    if self.config.is_cloud:
+                        # Cloud requires accountId
+                        if "accountId" in user:
+                            return user["accountId"]
+                    else:
+                        # Server/DC requires 'name' for the assignee field { "name": ... }
+                        if "name" in user:
+                            logger.info(
+                                "Using 'name' for assignee field in Jira Data Center/Server"
+                            )
+                            return user["name"]
+                        # Fallback to key if name is somehow missing (less common)
+                        elif "key" in user:
+                            logger.info(
+                                "Using 'key' as fallback for assignee name in Jira Data Center/Server"
+                            )
+                            return user["key"]
             return None
         except Exception as e:
             logger.info(f"Error looking up user by permissions: {str(e)}")

tests/unit/jira/test_users.py

@@ -256,6 +256,7 @@ def test_lookup_user_directly_server_dc(self, users_mixin):
         users_mixin.jira.user_find_by_user_string.return_value = [
             {
                 "key": "server-user-key",
+                "name": "server-user-name",
                 "displayName": "Test User",
                 "emailAddress": "test@example.com",
             }
@@ -268,7 +269,32 @@ def test_lookup_user_directly_server_dc(self, users_mixin):
         # Call the method
         account_id = users_mixin._lookup_user_directly("Test User")
 
-        # Verify result
+        # Verify result - should now return name instead of key for Server/DC
+        assert account_id == "server-user-name"
+        # Verify API call with username parameter for Server/DC
+        users_mixin.jira.user_find_by_user_string.assert_called_once_with(
+            username="Test User", start=0, limit=1
+        )
+
+    def test_lookup_user_directly_server_dc_key_fallback(self, users_mixin):
+        """Test _lookup_user_directly for Server/DC falls back to key when name is not available."""
+        # Mock the API response
+        users_mixin.jira.user_find_by_user_string.return_value = [
+            {
+                "key": "server-user-key",  # Only key, no name
+                "displayName": "Test User",
+                "emailAddress": "test@example.com",
+            }
+        ]
+
+        # Mock config.is_cloud to return False for Server/DC
+        users_mixin.config = MagicMock()
+        users_mixin.config.is_cloud = False
+
+        # Call the method
+        account_id = users_mixin._lookup_user_directly("Test User")
+
+        # Verify result - should fallback to key when name is missing
         assert account_id == "server-user-key"
         # Verify API call with username parameter for Server/DC
         users_mixin.jira.user_find_by_user_string.assert_called_once_with(
@@ -301,14 +327,18 @@ def test_lookup_user_directly_jira_data_center_key(self, users_mixin):
             }
         ]
 
+        # Mock config.is_cloud to return False for Server/DC
+        users_mixin.config = MagicMock()
+        users_mixin.config.is_cloud = False
+
         # Call the method
         account_id = users_mixin._lookup_user_directly("Test User")
 
         # Verify result
         assert account_id == "data-center-key"
         # Verify API call
         users_mixin.jira.user_find_by_user_string.assert_called_once_with(
-            query="Test User", start=0, limit=1
+            username="Test User", start=0, limit=1
         )
 
     def test_lookup_user_directly_jira_data_center_name(self, users_mixin):
@@ -322,14 +352,18 @@ def test_lookup_user_directly_jira_data_center_name(self, users_mixin):
             }
         ]
 
+        # Mock config.is_cloud to return False for Server/DC
+        users_mixin.config = MagicMock()
+        users_mixin.config.is_cloud = False
+
         # Call the method
         account_id = users_mixin._lookup_user_directly("Test User")
 
         # Verify result
         assert account_id == "data-center-name"
         # Verify API call
         users_mixin.jira.user_find_by_user_string.assert_called_once_with(
-            query="Test User", start=0, limit=1
+            username="Test User", start=0, limit=1
         )
 
     def test_lookup_user_directly_error(self, users_mixin):
@@ -382,22 +416,31 @@ def test_lookup_user_by_permissions_not_found(self, users_mixin):
             # Verify result
             assert account_id is None
 
-    def test_lookup_user_by_permissions_jira_data_center_key(self, users_mixin):
-        """Test _lookup_user_by_permissions when only 'key' is available (Data Center)."""
+    def test_lookup_user_by_permissions_jira_data_center(self, users_mixin):
+        """Test _lookup_user_by_permissions when both 'key' and 'name' are available (Data Center)."""
         # Mock requests.get
         with patch("requests.get") as mock_get:
             mock_response = MagicMock()
             mock_response.status_code = 200
             mock_response.json.return_value = {
-                "users": [{"key": "data-center-permissions-key"}]
+                "users": [
+                    {
+                        "key": "data-center-permissions-key",
+                        "name": "data-center-permissions-name",
+                    }
+                ]
             }
             mock_get.return_value = mock_response
 
+            # Mock config.is_cloud to return False for Server/DC
+            users_mixin.config = MagicMock()
+            users_mixin.config.is_cloud = False
+
             # Call the method
             account_id = users_mixin._lookup_user_by_permissions("username")
 
-            # Verify result
-            assert account_id == "data-center-permissions-key"
+            # Verify result - should prioritize name for Server/DC
+            assert account_id == "data-center-permissions-name"
             # Verify API call
             mock_get.assert_called_once()
             assert mock_get.call_args[0][0].endswith("/user/permission/search")
@@ -406,22 +449,28 @@ def test_lookup_user_by_permissions_jira_data_center_key(self, users_mixin):
                 "permissions": "BROWSE",
             }
 
-    def test_lookup_user_by_permissions_jira_data_center_name(self, users_mixin):
-        """Test _lookup_user_by_permissions when only 'name' is available (Data Center)."""
+    def test_lookup_user_by_permissions_jira_data_center_key_fallback(
+        self, users_mixin
+    ):
+        """Test _lookup_user_by_permissions when only 'key' is available (Data Center)."""
         # Mock requests.get
         with patch("requests.get") as mock_get:
             mock_response = MagicMock()
             mock_response.status_code = 200
             mock_response.json.return_value = {
-                "users": [{"name": "data-center-permissions-name"}]
+                "users": [{"key": "data-center-permissions-key"}]
             }
             mock_get.return_value = mock_response
 
+            # Mock config.is_cloud to return False for Server/DC
+            users_mixin.config = MagicMock()
+            users_mixin.config.is_cloud = False
+
             # Call the method
             account_id = users_mixin._lookup_user_by_permissions("username")
 
-            # Verify result
-            assert account_id == "data-center-permissions-name"
+            # Verify result - should fallback to key when name is missing
+            assert account_id == "data-center-permissions-key"
             # Verify API call
             mock_get.assert_called_once()
             assert mock_get.call_args[0][0].endswith("/user/permission/search")
@@ -439,3 +488,31 @@ def test_lookup_user_by_permissions_error(self, users_mixin):
 
             # Verify result
             assert account_id is None
+
+    def test_lookup_user_by_permissions_jira_data_center_name_only(self, users_mixin):
+        """Test _lookup_user_by_permissions when only 'name' is available (Data Center)."""
+        # Mock requests.get
+        with patch("requests.get") as mock_get:
+            mock_response = MagicMock()
+            mock_response.status_code = 200
+            mock_response.json.return_value = {
+                "users": [{"name": "data-center-permissions-name"}]
+            }
+            mock_get.return_value = mock_response
+
+            # Mock config.is_cloud to return False for Server/DC
+            users_mixin.config = MagicMock()
+            users_mixin.config.is_cloud = False
+
+            # Call the method
+            account_id = users_mixin._lookup_user_by_permissions("username")
+
+            # Verify result - should use name when that's all that's available
+            assert account_id == "data-center-permissions-name"
+            # Verify API call
+            mock_get.assert_called_once()
+            assert mock_get.call_args[0][0].endswith("/user/permission/search")
+            assert mock_get.call_args[1]["params"] == {
+                "query": "username",
+                "permissions": "BROWSE",
+            }