diff --git a/mcp_zammad/client.py b/mcp_zammad/client.py index 2439d09..989f791 100644 --- a/mcp_zammad/client.py +++ b/mcp_zammad/client.py @@ -217,6 +217,7 @@ def update_ticket( priority: str | None = None, owner: str | None = None, group: str | None = None, + time_unit: float | None = None, ) -> dict[str, Any]: """Update an existing ticket.""" update_data = {} @@ -230,6 +231,8 @@ def update_ticket( update_data["owner"] = owner if group is not None: update_data["group"] = group + if time_unit is not None: + update_data["time_unit"] = time_unit return dict(self.api.ticket.update(ticket_id, update_data)) @@ -240,6 +243,7 @@ def add_article( article_type: str = "note", internal: bool = False, sender: str = "Agent", + time_unit: float | None = None, attachments: list[dict[str, str]] | None = None, ) -> dict[str, Any]: """Add an article (comment/note) to a ticket with optional attachments. @@ -250,6 +254,7 @@ def add_article( article_type: Article type (note, email, phone) internal: Whether the article is internal sender: Sender type (Agent, Customer, System) + time_unit: Time spent for time accounting (unit defined in Zammad admin settings) attachments: Optional list of attachments with keys: - filename: str - data: str (base64-encoded content) @@ -266,6 +271,9 @@ def add_article( "sender": sender, } + if time_unit is not None: + article_data["time_unit"] = time_unit + if attachments: article_data["attachments"] = attachments diff --git a/mcp_zammad/models.py b/mcp_zammad/models.py index e5d7273..155ce58 100644 --- a/mcp_zammad/models.py +++ b/mcp_zammad/models.py @@ -280,6 +280,9 @@ class TicketUpdate(StrictBaseModel): priority: str | None = Field(None, description="New priority name", max_length=100) owner: str | None = Field(None, description="New owner login/email", max_length=255) group: str | None = Field(None, description="New group name", max_length=100) + time_unit: float | None = Field( + None, description="Time spent for time accounting (unit defined in Zammad admin settings)", gt=0 + ) @field_validator("title") @classmethod @@ -322,6 +325,9 @@ class ArticleCreate(StrictBaseModel): article_type: ArticleType = Field(default=ArticleType.NOTE, alias="type", description="Article type") internal: bool = Field(default=False, description="Whether the article is internal") sender: ArticleSender = Field(default=ArticleSender.AGENT, description="Sender type") + time_unit: float | None = Field( + default=None, description="Time spent for time accounting (unit defined in Zammad admin settings)", gt=0 + ) attachments: list[AttachmentUpload] | None = Field( default=None, description="Optional attachments to include", max_length=10 ) @@ -354,6 +360,9 @@ class TicketUpdateParams(StrictBaseModel): priority: str | None = Field(None, description="New priority name", max_length=100) owner: str | None = Field(None, description="New owner login/email", max_length=255) group: str | None = Field(None, description="New group name", max_length=100) + time_unit: float | None = Field( + None, description="Time spent for time accounting (unit defined in Zammad admin settings)", gt=0 + ) @field_validator("title") @classmethod diff --git a/mcp_zammad/server.py b/mcp_zammad/server.py index 28d928a..1b23164 100644 --- a/mcp_zammad/server.py +++ b/mcp_zammad/server.py @@ -1119,6 +1119,7 @@ def zammad_update_ticket(params: TicketUpdateParams) -> Ticket: - group (str | None): New group name - owner (str | None): New owner email/login - customer (str | None): New customer email/login + - time_unit (float | None): Time spent for time accounting Returns: Ticket: The updated ticket object with schema: diff --git a/tests/test_client.py b/tests/test_client.py index 4339ff6..de20741 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -185,6 +185,40 @@ def test_add_article_without_attachments_backward_compat(mock_api: MagicMock) -> assert "attachments" not in call_args # Should not include empty attachments +@patch("mcp_zammad.client.ZammadAPI") +def test_add_article_with_time_unit(mock_api: MagicMock) -> None: + """Test adding article with time_unit for time accounting.""" + mock_instance = mock_api.return_value + mock_instance.ticket_article.create.return_value = {"id": 789, "ticket_id": 123, "body": "Worked on this"} + + with patch.dict( + os.environ, {"ZAMMAD_URL": "https://test.zammad.com/api/v1", "ZAMMAD_HTTP_TOKEN": "token"}, clear=True + ): + client = ZammadClient() + result = client.add_article(ticket_id=123, body="Worked on this", time_unit=45.5) + + assert result["id"] == 789 + call_args = mock_instance.ticket_article.create.call_args[0][0] + assert call_args["time_unit"] == 45.5 + + +@patch("mcp_zammad.client.ZammadAPI") +def test_add_article_without_time_unit_excludes_field(mock_api: MagicMock) -> None: + """Test that time_unit is not included in payload when None.""" + mock_instance = mock_api.return_value + mock_instance.ticket_article.create.return_value = {"id": 789, "ticket_id": 123, "body": "Simple comment"} + + with patch.dict( + os.environ, {"ZAMMAD_URL": "https://test.zammad.com/api/v1", "ZAMMAD_HTTP_TOKEN": "token"}, clear=True + ): + client = ZammadClient() + result = client.add_article(ticket_id=123, body="Simple comment") + + assert result["id"] == 789 + call_args = mock_instance.ticket_article.create.call_args[0][0] + assert "time_unit" not in call_args + + @patch("mcp_zammad.client.ZammadAPI") def test_delete_attachment_success(mock_api: MagicMock) -> None: """Test successful attachment deletion.""" diff --git a/tests/test_client_methods.py b/tests/test_client_methods.py index ffbacaa..6930330 100644 --- a/tests/test_client_methods.py +++ b/tests/test_client_methods.py @@ -71,6 +71,32 @@ def test_update_ticket(self, mock_zammad_api: Mock) -> None: assert result["title"] == "Updated Title" mock_instance.ticket.update.assert_called_once_with(1, {"title": "Updated Title", "state": "open"}) + def test_update_ticket_with_time_unit(self, mock_zammad_api: Mock) -> None: + """Test update_ticket method with time_unit for time accounting.""" + mock_instance = Mock() + mock_instance.ticket.update.return_value = {"id": 1, "title": "Test", "state": "open"} + mock_zammad_api.return_value = mock_instance + + client = ZammadClient(url="https://test.zammad.com/api/v1", http_token="test-token") + + result = client.update_ticket(1, title="Test", time_unit=2.5) + + assert result["id"] == 1 + mock_instance.ticket.update.assert_called_once_with(1, {"title": "Test", "time_unit": 2.5}) + + def test_update_ticket_without_time_unit_excludes_field(self, mock_zammad_api: Mock) -> None: + """Test that time_unit is not included in payload when None.""" + mock_instance = Mock() + mock_instance.ticket.update.return_value = {"id": 1, "title": "Test"} + mock_zammad_api.return_value = mock_instance + + client = ZammadClient(url="https://test.zammad.com/api/v1", http_token="test-token") + + client.update_ticket(1, title="Test") + + call_args = mock_instance.ticket.update.call_args[0][1] + assert "time_unit" not in call_args + def test_get_groups(self, mock_zammad_api: Mock) -> None: """Test get_groups method.""" mock_instance = Mock() @@ -366,6 +392,35 @@ def test_add_article(self, mock_zammad_api: Mock) -> None: {"ticket_id": 1, "body": "Response", "type": "email", "internal": True, "sender": "Customer"} ) + def test_add_article_with_time_unit(self, mock_zammad_api: Mock) -> None: + """Test add_article method with time_unit.""" + mock_instance = Mock() + mock_instance.ticket_article.create.return_value = {"id": 2, "body": "Worked on it", "type": "note"} + mock_zammad_api.return_value = mock_instance + + client = ZammadClient(url="https://test.zammad.com/api/v1", http_token="test-token") + + result = client.add_article(ticket_id=1, body="Worked on it", time_unit=15.0) + + assert result["id"] == 2 + mock_instance.ticket_article.create.assert_called_once_with( + {"ticket_id": 1, "body": "Worked on it", "type": "note", "internal": False, "sender": "Agent", "time_unit": 15.0} + ) + + def test_add_article_without_time_unit(self, mock_zammad_api: Mock) -> None: + """Test add_article method without time_unit excludes it from payload.""" + mock_instance = Mock() + mock_instance.ticket_article.create.return_value = {"id": 3, "body": "Simple note", "type": "note"} + mock_zammad_api.return_value = mock_instance + + client = ZammadClient(url="https://test.zammad.com/api/v1", http_token="test-token") + + result = client.add_article(ticket_id=1, body="Simple note") + + assert result["id"] == 3 + call_args = mock_instance.ticket_article.create.call_args[0][0] + assert "time_unit" not in call_args + def test_get_user(self, mock_zammad_api: Mock) -> None: """Test get_user method.""" mock_instance = Mock() diff --git a/tests/test_server.py b/tests/test_server.py index c5ecd20..6c4442b 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -37,6 +37,7 @@ TicketPriority, TicketSearchParams, TicketState, + TicketUpdateParams, User, UserBrief, UserCreate, @@ -517,10 +518,73 @@ def test_add_article_tool(mock_zammad_client, sample_article_data, decorator_cap # Verify the client was called with correct params (including attachments=None for backward compat) mock_instance.add_article.assert_called_once_with( - ticket_id=1, article_type="note", attachments=None, body="New comment", internal=False, sender="Agent" + ticket_id=1, + article_type="note", + attachments=None, + body="New comment", + internal=False, + sender="Agent", + time_unit=None, ) +def test_add_article_with_time_unit_tool(mock_zammad_client, sample_article_data, decorator_capturer): + """Test zammad_add_article tool with time_unit for time accounting.""" + mock_instance, _ = mock_zammad_client + + mock_instance.add_article.return_value = sample_article_data + + server_inst = ZammadMCPServer() + server_inst.client = mock_instance + + test_tools, capture_tool = decorator_capturer(server_inst.mcp.tool) + server_inst.mcp.tool = capture_tool # type: ignore[method-assign, assignment] + server_inst.get_client = lambda: server_inst.client # type: ignore[method-assign, assignment, return-value] + server_inst._setup_tools() + + params = ArticleCreate(ticket_id=1, body="Worked on this issue", time_unit=30.5) + result = test_tools["zammad_add_article"](params) + + assert result.body == "Test article" + + mock_instance.add_article.assert_called_once() + call_kwargs = mock_instance.add_article.call_args[1] + assert call_kwargs["time_unit"] == 30.5 + + +def test_add_article_without_time_unit_tool(mock_zammad_client, sample_article_data, decorator_capturer): + """Test zammad_add_article tool without time_unit passes None.""" + mock_instance, _ = mock_zammad_client + + mock_instance.add_article.return_value = sample_article_data + + server_inst = ZammadMCPServer() + server_inst.client = mock_instance + + test_tools, capture_tool = decorator_capturer(server_inst.mcp.tool) + server_inst.mcp.tool = capture_tool # type: ignore[method-assign, assignment] + server_inst.get_client = lambda: server_inst.client # type: ignore[method-assign, assignment, return-value] + server_inst._setup_tools() + + params = ArticleCreate(ticket_id=1, body="Simple comment") + result = test_tools["zammad_add_article"](params) + + assert result.body == "Test article" + + mock_instance.add_article.assert_called_once() + call_kwargs = mock_instance.add_article.call_args[1] + assert call_kwargs["time_unit"] is None + + +def test_add_article_invalid_time_unit(): + """Test that ArticleCreate rejects invalid time_unit values.""" + with pytest.raises(ValidationError, match="time_unit"): + ArticleCreate(ticket_id=1, body="test", time_unit=0) + + with pytest.raises(ValidationError, match="time_unit"): + ArticleCreate(ticket_id=1, body="test", time_unit=-5) + + def test_add_article_invalid_type(): """Test that ArticleCreate rejects invalid article types.""" # Test invalid article type @@ -718,6 +782,59 @@ def test_update_ticket_tool(mock_zammad_client, sample_ticket_data): ) +def test_update_ticket_with_time_unit_tool(mock_zammad_client, sample_ticket_data): + """Test update ticket tool with time_unit for time accounting.""" + mock_instance, _ = mock_zammad_client + + updated_ticket = sample_ticket_data.copy() + updated_ticket["title"] = "Updated Title" + + mock_instance.update_ticket.return_value = updated_ticket + + server_inst = ZammadMCPServer() + server_inst.client = mock_instance + client = server_inst.get_client() + + ticket_data = client.update_ticket(1, title="Updated Title", time_unit=2.5) + result = Ticket(**ticket_data) + + assert result.id == 1 + mock_instance.update_ticket.assert_called_once_with(1, title="Updated Title", time_unit=2.5) + + +def test_update_ticket_without_time_unit_tool(mock_zammad_client, sample_ticket_data): + """Test update ticket tool without time_unit does not pass it.""" + mock_instance, _ = mock_zammad_client + + mock_instance.update_ticket.return_value = sample_ticket_data + + server_inst = ZammadMCPServer() + server_inst.client = mock_instance + client = server_inst.get_client() + + client.update_ticket(1, title="Updated Title") + + mock_instance.update_ticket.assert_called_once_with(1, title="Updated Title") + + +def test_update_ticket_invalid_time_unit(): + """Test that TicketUpdateParams rejects invalid time_unit values.""" + with pytest.raises(ValidationError, match="time_unit"): + TicketUpdateParams(ticket_id=1, time_unit=0) + + with pytest.raises(ValidationError, match="time_unit"): + TicketUpdateParams(ticket_id=1, time_unit=-5) + + +def test_update_ticket_valid_time_unit(): + """Test that TicketUpdateParams accepts valid time_unit values.""" + params = TicketUpdateParams(ticket_id=1, time_unit=1.5) + assert params.time_unit == 1.5 + + params_none = TicketUpdateParams(ticket_id=1) + assert params_none.time_unit is None + + def test_get_organization_tool(mock_zammad_client, sample_organization_data): """Test get organization tool.""" mock_instance, _ = mock_zammad_client