Skip to content

MPT-18971 introduce chat attachments module with sync and async support#236

Merged
jentyk merged 1 commit intomainfrom
feat/MPT-18971
Mar 19, 2026
Merged

MPT-18971 introduce chat attachments module with sync and async support#236
jentyk merged 1 commit intomainfrom
feat/MPT-18971

Conversation

@jentyk
Copy link
Member

@jentyk jentyk commented Mar 18, 2026

Closes MPT-18971

  • Added new ChatAttachment model and ChatAttachmentsService/AsyncChatAttachmentsService classes to provide file attachment operations on helpdesk chats with support for CRUD operations, file upload/download, and pagination
  • Extended ChatsService and AsyncChatsService with attachments(chat_id) methods to access attachment services for a specific chat
  • Added comprehensive end-to-end tests for both sync and async attachment operations including create, read, update, delete, download, and error handling scenarios
  • Added unit tests validating service instantiation and method availability for both synchronous and asynchronous service implementations
  • Introduced pytest fixtures for attachment service setup, test data generation, and attachment lifecycle management (creation and cleanup)

@coderabbitai
Copy link

coderabbitai bot commented Mar 18, 2026

📝 Walkthrough

Walkthrough

Added a new Chat Attachments resource module for Helpdesk, consisting of ChatAttachment model and synchronous/asynchronous service classes with full CRUD operations. Extended the Chats service to provide attachment accessors. Added comprehensive unit and end-to-end tests covering service instantiation, file operations, pagination, and error handling.

Changes

Cohort / File(s) Summary
Chat Attachments Resource
mpt_api_client/resources/helpdesk/chat_attachments.py
Introduced ChatAttachment model and ChatAttachmentsService/AsyncChatAttachmentsService composed of CRUD mixins. Configured endpoint /public/v1/helpdesk/chats/{chat_id}/attachments, file upload key file with request key attachment, and collection key data.
Chats Service Extensions
mpt_api_client/resources/helpdesk/chats.py
Added attachments(chat_id) methods to ChatsService and AsyncChatsService to instantiate and return corresponding attachment services with proper endpoint parameters.
Unit Tests
tests/unit/resources/helpdesk/test_chat_attachments.py, tests/unit/resources/helpdesk/test_chats.py
Added tests verifying service instantiation, endpoint path resolution, method existence (get, create, update, delete, iterate, download), and endpoint parameter binding for attachment services.
End-to-End Tests
tests/e2e/helpdesk/chats/attachment/conftest.py, tests/e2e/helpdesk/chats/attachment/test_sync_attachment.py, tests/e2e/helpdesk/chats/attachment/test_async_attachment.py
Added fixtures for service instantiation and lifecycle management. Comprehensive test coverage for CRUD operations, file downloads, pagination, and 404 error handling for both sync and async paths.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • MPT-18371 Helpdesk Chats core #227: Introduces the foundational Helpdesk Chats feature that this PR extends with Chat Attachments resource and attachment accessors on the same Chats service classes.
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Jira Issue Key In Title ✅ Passed The PR title contains exactly one Jira issue key (MPT-18971) in the required MPT-XXXX format.
Test Coverage Required ✅ Passed The PR modifies 2 code files and includes comprehensive test coverage with 5 test files covering new chat attachments functionality.
Single Commit Required ✅ Passed The PR contains exactly one commit (f2682b2 feat(helpdesk): introduce chat attachments module with sync and async support), as confirmed by git log, satisfying the requirement for a clean git history.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

📝 Coding Plan
  • Generate coding plan for human review comments

Comment @coderabbitai help to get the list of available commands and usage tips.

@jentyk jentyk marked this pull request as ready for review March 19, 2026 16:59
@jentyk jentyk requested a review from a team as a code owner March 19, 2026 16:59
@jentyk jentyk requested review from alephsur and svazquezco March 19, 2026 16:59
@sonarqubecloud
Copy link

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
tests/unit/resources/helpdesk/test_chat_attachments.py (1)

23-37: Consider simplifying the assertions.

The pattern of assigning a comparison to result and then asserting result is True is more verbose than necessary. A direct assertion would be clearer:

assert chat_attachments_service.path == expected_path

However, this pattern appears intentional and consistent with other tests in the codebase, so this is purely optional.

♻️ Optional simplification
 def test_endpoint(chat_attachments_service) -> None:
     expected_path = "/public/v1/helpdesk/chats/CHT-0000-0000-0001/attachments"
 
-    result = chat_attachments_service.path == expected_path
-
-    assert result is True
+    assert chat_attachments_service.path == expected_path
 
 
 def test_async_endpoint(async_chat_attachments_service) -> None:
-    result = (
-        async_chat_attachments_service.path
-        == "/public/v1/helpdesk/chats/CHT-0000-0000-0001/attachments"
-    )
-
-    assert result is True
+    assert async_chat_attachments_service.path == "/public/v1/helpdesk/chats/CHT-0000-0000-0001/attachments"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/unit/resources/helpdesk/test_chat_attachments.py` around lines 23 - 37,
Replace the verbose assertion pattern in test_endpoint and test_async_endpoint
by asserting the comparison directly: instead of assigning the boolean to result
and asserting result is True, call assert chat_attachments_service.path ==
expected_path and assert async_chat_attachments_service.path ==
"/public/v1/helpdesk/chats/CHT-0000-0000-0001/attachments" respectively; update
the tests named test_endpoint and test_async_endpoint to remove the intermediate
result variable and assert the equality in one line each.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@tests/unit/resources/helpdesk/test_chat_attachments.py`:
- Around line 23-37: Replace the verbose assertion pattern in test_endpoint and
test_async_endpoint by asserting the comparison directly: instead of assigning
the boolean to result and asserting result is True, call assert
chat_attachments_service.path == expected_path and assert
async_chat_attachments_service.path ==
"/public/v1/helpdesk/chats/CHT-0000-0000-0001/attachments" respectively; update
the tests named test_endpoint and test_async_endpoint to remove the intermediate
result variable and assert the equality in one line each.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: e1c4f8bc-b753-4f1b-81b6-f972002df516

📥 Commits

Reviewing files that changed from the base of the PR and between ec60985 and f2682b2.

📒 Files selected for processing (8)
  • mpt_api_client/resources/helpdesk/chat_attachments.py
  • mpt_api_client/resources/helpdesk/chats.py
  • tests/e2e/helpdesk/chats/attachment/__init__.py
  • tests/e2e/helpdesk/chats/attachment/conftest.py
  • tests/e2e/helpdesk/chats/attachment/test_async_attachment.py
  • tests/e2e/helpdesk/chats/attachment/test_sync_attachment.py
  • tests/unit/resources/helpdesk/test_chat_attachments.py
  • tests/unit/resources/helpdesk/test_chats.py

@jentyk jentyk merged commit d5b28dc into main Mar 19, 2026
4 checks passed
@jentyk jentyk deleted the feat/MPT-18971 branch March 19, 2026 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants