Merge PR sooperset#685: fix: make jira_get_sprint_issues respect fields parameter

Author: nszceta

src/mcp_atlassian/jira/search.py

@@ -255,36 +255,17 @@ def get_sprint_issues(
             JiraSearchResult object containing sprint issues and metadata
 
         Raises:
-            Exception: If there is an error getting board issues
+            Exception: If there is an error getting sprint issues
         """
         try:
-            # Determine fields_param
-            fields_param = fields
-            if fields_param is None:
-                fields_param = ",".join(DEFAULT_READ_JIRA_FIELDS)
-
-            response = self.jira.get_sprint_issues(
-                sprint_id=sprint_id,
+            # Use JQL search to get sprint issues with proper fields filtering
+            jql = f"sprint = {sprint_id}"
+            return self.search_issues(
+                jql=jql,
+                fields=fields,
                 start=start,
                 limit=limit,
             )
-            if not isinstance(response, dict):
-                msg = f"Unexpected return value type from `jira.get_sprint_issues`: {type(response)}"
-                logger.error(msg)
-                raise TypeError(msg)
-
-            # Convert the response to a search result model
-            search_result = JiraSearchResult.from_api_response(
-                response, base_url=self.config.url, requested_fields=fields_param
-            )
-            return search_result
-        except requests.HTTPError as e:
-            logger.error(
-                f"Error searching issues for sprint '{sprint_id}': {str(e.response.content)}"
-            )
-            raise Exception(
-                f"Error searching issues for sprint: {str(e.response.content)}"
-            ) from e
         except Exception as e:
-            logger.error(f"Error searching issues for sprint: {sprint_id}': {str(e)}")
+            logger.error(f"Error searching issues for sprint '{sprint_id}': {str(e)}")
             raise Exception(f"Error searching issues for sprint: {str(e)}") from e

tests/unit/jira/test_search.py

@@ -527,11 +527,24 @@ def test_get_sprint_issues(self, search_mixin: SearchMixin):
             "startAt": 0,
             "maxResults": 50,
         }
-        search_mixin.jira.get_sprint_issues.return_value = mock_issues
+
+        # Mock search_issues since get_sprint_issues now uses it internally
+        search_result = JiraSearchResult.from_api_response(
+            mock_issues, base_url=search_mixin.config.url
+        )
+        search_mixin.search_issues = MagicMock(return_value=search_result)
 
         # Call the method
         result = search_mixin.get_sprint_issues("10001")
 
+        # Verify that search_issues was called with correct JQL
+        search_mixin.search_issues.assert_called_once_with(
+            jql="sprint = 10001",
+            fields=None,
+            start=0,
+            limit=50,
+        )
+
         # Verify results
         assert isinstance(result, JiraSearchResult)
         assert len(result.issues) == 1
@@ -553,20 +566,91 @@ def test_get_sprint_issues(self, search_mixin: SearchMixin):
         assert issue.priority.name == "High"
 
     def test_get_sprint_issues_exception(self, search_mixin: SearchMixin):
-        search_mixin.jira.get_sprint_issues.side_effect = Exception("API Error")
+        search_mixin.search_issues = MagicMock(side_effect=Exception("API Error"))
 
         with pytest.raises(Exception) as e:
             search_mixin.get_sprint_issues("10001")
         assert "API Error" in str(e.value)
 
     def test_get_sprint_issues_http_error(self, search_mixin: SearchMixin):
-        search_mixin.jira.get_sprint_issues.side_effect = requests.HTTPError(
-            response=MagicMock(content="API Error content")
+        search_mixin.search_issues = MagicMock(
+            side_effect=requests.HTTPError(
+                response=MagicMock(content="API Error content")
+            )
         )
 
         with pytest.raises(Exception) as e:
             search_mixin.get_sprint_issues("10001")
-        assert "API Error content" in str(e.value)
+        assert "Error searching issues for sprint" in str(e.value)
+
+    def test_get_sprint_issues_with_fields_parameter(self, search_mixin: SearchMixin):
+        """Test get_sprint_issues method properly passes fields parameter to search_issues."""
+        mock_issues = {
+            "issues": [
+                {
+                    "id": "10001",
+                    "key": "TEST-123",
+                    "fields": {
+                        "summary": "Test issue with custom field",
+                        "assignee": {
+                            "displayName": "Test User",
+                            "emailAddress": "[email protected]",
+                            "active": True,
+                        },
+                        "customfield_10049": "Custom value",
+                        "issuetype": {"name": "Bug"},
+                        "status": {"name": "Open"},
+                        "description": "Issue description",
+                        "created": "2024-01-01T10:00:00.000+0000",
+                        "updated": "2024-01-01T11:00:00.000+0000",
+                        "priority": {"name": "High"},
+                    },
+                }
+            ],
+            "total": 1,
+            "startAt": 0,
+            "maxResults": 50,
+        }
+
+        # Mock search_issues to return a result with requested_fields set
+        search_result = JiraSearchResult.from_api_response(
+            mock_issues,
+            base_url=search_mixin.config.url,
+            requested_fields="summary,assignee,customfield_10049",
+        )
+        search_mixin.search_issues = MagicMock(return_value=search_result)
+
+        # Call the method with specific fields
+        result = search_mixin.get_sprint_issues(
+            "10001", fields="summary,assignee,customfield_10049"
+        )
+
+        # Verify that search_issues was called with correct parameters
+        search_mixin.search_issues.assert_called_once_with(
+            jql="sprint = 10001",
+            fields="summary,assignee,customfield_10049",
+            start=0,
+            limit=50,
+        )
+
+        # Verify results
+        assert isinstance(result, JiraSearchResult)
+        assert len(result.issues) == 1
+        issue = result.issues[0]
+
+        # Convert to simplified dict to check field filtering
+        simplified = issue.to_simplified_dict()
+
+        # These fields should be included (plus id and key which are always included)
+        assert "id" in simplified
+        assert "key" in simplified
+        assert "summary" in simplified
+        assert "assignee" in simplified
+        assert "customfield_10049" in simplified
+
+        assert simplified["customfield_10049"] == {"value": "Custom value"}
+        assert "assignee" in simplified
+        assert simplified["assignee"]["display_name"] == "Test User"
 
     @pytest.mark.parametrize("is_cloud", [True, False])
     def test_search_issues_with_projects_filter_jql_construction(