MPT-18971 introduce chat attachments module with sync and async support#236
MPT-18971 introduce chat attachments module with sync and async support#236
Conversation
📝 WalkthroughWalkthroughAdded a new Chat Attachments resource module for Helpdesk, consisting of Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
Comment |
|
There was a problem hiding this comment.
🧹 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
resultand then assertingresult is Trueis more verbose than necessary. A direct assertion would be clearer:assert chat_attachments_service.path == expected_pathHowever, 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
📒 Files selected for processing (8)
mpt_api_client/resources/helpdesk/chat_attachments.pympt_api_client/resources/helpdesk/chats.pytests/e2e/helpdesk/chats/attachment/__init__.pytests/e2e/helpdesk/chats/attachment/conftest.pytests/e2e/helpdesk/chats/attachment/test_async_attachment.pytests/e2e/helpdesk/chats/attachment/test_sync_attachment.pytests/unit/resources/helpdesk/test_chat_attachments.pytests/unit/resources/helpdesk/test_chats.py



Closes MPT-18971
ChatAttachmentmodel andChatAttachmentsService/AsyncChatAttachmentsServiceclasses to provide file attachment operations on helpdesk chats with support for CRUD operations, file upload/download, and paginationChatsServiceandAsyncChatsServicewithattachments(chat_id)methods to access attachment services for a specific chat