Add direct message and file upload methods to GroupMe SDK #8

Merged

forgejo_admin merged 2 commits from 7-add-direct-message-and-file-upload-metho into main

2026-03-28 19:26:18 +00:00

forgejo_admin commented

2026-03-28 19:19:45 +00:00

Owner

Summary

Adds DirectMessagesMixin and FilesMixin to the GroupMe SDK, exposing DM send/list and file upload/status endpoints following the established mixin pattern. File operations target file.groupme.com with a dedicated request handler that shares error-handling semantics with the main API client.

Changes

src/groupme_sdk/direct_messages.py -- new DirectMessagesMixin with send_dm(recipient_id, text) and list_chats(page, per_page)
src/groupme_sdk/files.py -- new FilesMixin with upload_file(group_id, file_path, content_type) and get_upload_status(group_id, job_id), plus _do_file_request for file.groupme.com requests
src/groupme_sdk/client.py -- added DirectMessagesMixin and FilesMixin to GroupMeClient inheritance chain
tests/test_direct_messages.py -- 11 unit tests covering send_dm and list_chats
tests/test_files.py -- 12 unit tests covering upload_file and get_upload_status

Test Plan

All 59 unit tests pass (23 new + 36 existing), 1 integration test skipped (requires live token)
Ruff format and check clean
Verified MRO includes all mixins without conflicts
Tests cover: happy path, pagination, error handling (404, 500, network), file not found, auto content-type detection, request body/header/URL validation

Review Checklist

Code follows existing mixin pattern (TYPE_CHECKING import, self: GroupMeClient annotation)
No new dependencies added (pure stdlib: urllib, mimetypes, os, uuid, json)
All new methods have docstrings with Args/Returns/Raises sections
Error handling consistent with existing _request method
Tests use shared conftest helpers (make_client, mock_response, mock_http_error)
Ruff format clean
Ruff check clean

Upstream dependency for groupme-mcp#7 (DM and file upload MCP tools)
Board: board-westside-basketball

Closes #7

## Summary Adds DirectMessagesMixin and FilesMixin to the GroupMe SDK, exposing DM send/list and file upload/status endpoints following the established mixin pattern. File operations target file.groupme.com with a dedicated request handler that shares error-handling semantics with the main API client. ## Changes - `src/groupme_sdk/direct_messages.py` -- new DirectMessagesMixin with `send_dm(recipient_id, text)` and `list_chats(page, per_page)` - `src/groupme_sdk/files.py` -- new FilesMixin with `upload_file(group_id, file_path, content_type)` and `get_upload_status(group_id, job_id)`, plus `_do_file_request` for file.groupme.com requests - `src/groupme_sdk/client.py` -- added DirectMessagesMixin and FilesMixin to GroupMeClient inheritance chain - `tests/test_direct_messages.py` -- 11 unit tests covering send_dm and list_chats - `tests/test_files.py` -- 12 unit tests covering upload_file and get_upload_status ## Test Plan - All 59 unit tests pass (23 new + 36 existing), 1 integration test skipped (requires live token) - Ruff format and check clean - Verified MRO includes all mixins without conflicts - Tests cover: happy path, pagination, error handling (404, 500, network), file not found, auto content-type detection, request body/header/URL validation ## Review Checklist - [x] Code follows existing mixin pattern (TYPE_CHECKING import, self: GroupMeClient annotation) - [x] No new dependencies added (pure stdlib: urllib, mimetypes, os, uuid, json) - [x] All new methods have docstrings with Args/Returns/Raises sections - [x] Error handling consistent with existing _request method - [x] Tests use shared conftest helpers (make_client, mock_response, mock_http_error) - [x] Ruff format clean - [x] Ruff check clean ## Related Notes - Upstream dependency for groupme-mcp#7 (DM and file upload MCP tools) - Board: board-westside-basketball ## Related Closes #7

forgejo_admin added 1 commit

2026-03-28 19:19:45 +00:00

feat: add direct message and file upload methods to GroupMe SDK 759829fc61

New DirectMessagesMixin (send_dm, list_chats) and FilesMixin
(upload_file, get_upload_status) following existing mixin pattern.
File operations target file.groupme.com with separate request handler.
23 new tests, all passing.

Closes #7

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin added 1 commit

2026-03-28 19:21:11 +00:00

fix: move imports to module level in files.py for pattern consistency a3d955aa7a

Lazy imports inside _do_file_request (json, urllib.error, exceptions)
moved to module-level to match client.py convention.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin commented

2026-03-28 19:21:28 +00:00

Author

Owner

QA Review -- PR #8

Pattern Compliance

DirectMessagesMixin follows the exact same pattern as GroupsMixin/MessagesMixin: TYPE_CHECKING guard, self: GroupMeClient annotation, docstrings with Args/Returns/Raises sections. PASS.
FilesMixin correctly separates file.groupme.com requests from the main api.groupme.com base via _do_file_request. PASS.
Client MRO verified: all 5 mixins resolve without conflicts. PASS.

Issue Found and Fixed

Lazy imports in _do_file_request: json, urllib.error, and exception classes were imported inside the method body. The existing pattern in client.py uses module-level imports. Fixed in commit a3d955a -- moved all imports to module top level.

Test Coverage

23 new tests across 2 files (11 DM, 12 files). All use existing conftest helpers.
Covers: happy path, pagination, request body/header/URL validation, error handling (404, 500, network), file-not-found guard, auto content-type detection.
No test gaps identified.

Ruff

ruff format -- clean
ruff check -- clean

VERDICT: APPROVED

## QA Review -- PR #8 ### Pattern Compliance - DirectMessagesMixin follows the exact same pattern as GroupsMixin/MessagesMixin: `TYPE_CHECKING` guard, `self: GroupMeClient` annotation, docstrings with Args/Returns/Raises sections. PASS. - FilesMixin correctly separates `file.groupme.com` requests from the main `api.groupme.com` base via `_do_file_request`. PASS. - Client MRO verified: all 5 mixins resolve without conflicts. PASS. ### Issue Found and Fixed - **Lazy imports in `_do_file_request`**: `json`, `urllib.error`, and exception classes were imported inside the method body. The existing pattern in `client.py` uses module-level imports. Fixed in commit a3d955a -- moved all imports to module top level. ### Test Coverage - 23 new tests across 2 files (11 DM, 12 files). All use existing conftest helpers. - Covers: happy path, pagination, request body/header/URL validation, error handling (404, 500, network), file-not-found guard, auto content-type detection. - No test gaps identified. ### Ruff - `ruff format` -- clean - `ruff check` -- clean ### VERDICT: APPROVED

forgejo_admin commented

2026-03-28 19:24:01 +00:00

Author

Owner

PR #8 Review

DOMAIN REVIEW

Tech stack: Pure Python SDK (stdlib only: urllib, json, mimetypes, os, uuid). No frameworks. Mixin inheritance pattern for composing GroupMeClient.

Pattern conformance: Both new mixins (DirectMessagesMixin, FilesMixin) follow the established mixin pattern exactly:

from __future__ import annotations + TYPE_CHECKING guard for circular import avoidance
self: GroupMeClient annotation on every method
Module-level docstrings, Google-style docstrings with Args/Returns/Raises
Consistent use of _request() for standard API endpoints and a new _do_file_request() for the file.groupme.com domain

DRY analysis: _do_file_request (files.py lines 100-112) mirrors the error-handling logic from _request (client.py lines 65-78). This is intentional -- the response parsing differs (_request unwraps a {"response": ...} envelope; _do_file_request returns raw JSON because file.groupme.com does not use the envelope). The error-handling duplication (HTTPError/URLError mapping) is in the HTTP transport layer, not the auth path. Token injection happens per-method via self.token, which is the same single-source pattern used everywhere. Acceptable tradeoff for a 12-line private method. If the file API grows beyond 2-3 methods, extracting a shared _handle_http_errors helper would be warranted.

PEP compliance: PEP 8 naming, PEP 484 type hints on all signatures, PEP 257 docstrings present on every public method and class. Line length within the 120 configured in ruff.

Input validation: upload_file validates file_path existence via os.path.isfile() before any I/O -- good. String parameters (group_id, recipient_id, job_id) are not validated for empty/None, but this is consistent with every existing mixin in the codebase (groups.py, members.py, messages.py all pass string IDs directly to URL construction). No regression.

Error handling: All three exception types (GroupMeNotFoundError, GroupMeHTTPError, GroupMeError) properly raised with from e chaining. Empty response body handled (returns {}). Non-list response from list_chats handled (returns []), matching the list_groups pattern exactly.

Test coverage (23 new tests):

test_direct_messages.py (11 tests): send_dm happy path, source_guid presence, recipient_id in body, correct endpoint URL, 404 error, 500 error. list_chats happy path, pagination params, empty list, non-list defensive return, server error.
test_files.py (12 tests): upload correct URL, header validation (token, content-type, filename), auto content-type detection, file data transmission, local FileNotFoundError, 404 error, 500 error. get_upload_status happy path, correct URL, token header, 404 error, network error.
Tests correctly use mock_file_response (no envelope) for file.groupme.com endpoints and the shared mock_response (with envelope) for standard API endpoints. This validates the envelope-vs-raw parsing difference.
All temp files properly cleaned up in finally blocks.

BLOCKERS

None.

NITS

Memory for large files (files.py line 54): file_data = f.read() loads the entire file into memory. For a messaging SDK this is fine in practice, but a max_size guard or a note in the docstring about file size limits would be defensive. Non-blocking.
Truncated branch name: Branch is 7-add-direct-message-and-file-upload-metho (truncated). Likely Forgejo auto-generation. Cosmetic only.
__init__.py exports: The new DirectMessagesMixin and FilesMixin are not exported in __init__.py. The existing mixins (GroupsMixin, MembersMixin, MessagesMixin) are also not exported -- only GroupMeClient and exceptions are. This is consistent. Consumers access methods through GroupMeClient, not mixins directly. No change needed.
Version bump: pyproject.toml is still at 0.2.0. If this SDK is published to Forgejo PyPI (like pal-e-docs-sdk), a version bump to 0.3.0 would be needed before publish. This may be handled in a separate chore PR (matching the pattern of PR #6 "chore: bump version to 0.2.0"). Non-blocking for this PR.

SOP COMPLIANCE

Branch named after issue (7-add-direct-message-and-file-upload-metho maps to issue #7)
PR body has: Summary, Changes, Test Plan, Review Checklist, Related
Related section references parent issue (Closes #7) and downstream dependency (groupme-mcp#7)
Board reference present (board-westside-basketball)
No secrets committed (token is fake-token in tests via conftest helper)
No unnecessary file changes (5 files, all directly related to the feature)
Tests exist with comprehensive coverage (23 new tests, happy path + error + edge cases)

PROCESS OBSERVATIONS

Clean additive PR -- 523 additions, 1 deletion (the client class signature change). Zero risk of regression to existing functionality.
Deployment frequency: this unblocks groupme-mcp#7 downstream. Good dependency chain documentation in the PR body.
The version bump for publish should be tracked as a follow-up chore issue to avoid scope creep here.
Test-to-code ratio is healthy: 359 lines of tests for 161 lines of implementation.

VERDICT: APPROVED

## PR #8 Review ### DOMAIN REVIEW **Tech stack**: Pure Python SDK (stdlib only: urllib, json, mimetypes, os, uuid). No frameworks. Mixin inheritance pattern for composing `GroupMeClient`. **Pattern conformance**: Both new mixins (`DirectMessagesMixin`, `FilesMixin`) follow the established mixin pattern exactly: - `from __future__ import annotations` + `TYPE_CHECKING` guard for circular import avoidance - `self: GroupMeClient` annotation on every method - Module-level docstrings, Google-style docstrings with Args/Returns/Raises - Consistent use of `_request()` for standard API endpoints and a new `_do_file_request()` for the file.groupme.com domain **DRY analysis**: `_do_file_request` (files.py lines 100-112) mirrors the error-handling logic from `_request` (client.py lines 65-78). This is intentional -- the response parsing differs (`_request` unwraps a `{"response": ...}` envelope; `_do_file_request` returns raw JSON because file.groupme.com does not use the envelope). The error-handling duplication (HTTPError/URLError mapping) is in the HTTP transport layer, not the auth path. Token injection happens per-method via `self.token`, which is the same single-source pattern used everywhere. Acceptable tradeoff for a 12-line private method. If the file API grows beyond 2-3 methods, extracting a shared `_handle_http_errors` helper would be warranted. **PEP compliance**: PEP 8 naming, PEP 484 type hints on all signatures, PEP 257 docstrings present on every public method and class. Line length within the 120 configured in ruff. **Input validation**: `upload_file` validates `file_path` existence via `os.path.isfile()` before any I/O -- good. String parameters (`group_id`, `recipient_id`, `job_id`) are not validated for empty/None, but this is consistent with every existing mixin in the codebase (groups.py, members.py, messages.py all pass string IDs directly to URL construction). No regression. **Error handling**: All three exception types (`GroupMeNotFoundError`, `GroupMeHTTPError`, `GroupMeError`) properly raised with `from e` chaining. Empty response body handled (returns `{}`). Non-list response from `list_chats` handled (returns `[]`), matching the `list_groups` pattern exactly. **Test coverage (23 new tests)**: - `test_direct_messages.py` (11 tests): send_dm happy path, source_guid presence, recipient_id in body, correct endpoint URL, 404 error, 500 error. list_chats happy path, pagination params, empty list, non-list defensive return, server error. - `test_files.py` (12 tests): upload correct URL, header validation (token, content-type, filename), auto content-type detection, file data transmission, local FileNotFoundError, 404 error, 500 error. get_upload_status happy path, correct URL, token header, 404 error, network error. - Tests correctly use `mock_file_response` (no envelope) for file.groupme.com endpoints and the shared `mock_response` (with envelope) for standard API endpoints. This validates the envelope-vs-raw parsing difference. - All temp files properly cleaned up in `finally` blocks. ### BLOCKERS None. ### NITS 1. **Memory for large files** (`files.py` line 54): `file_data = f.read()` loads the entire file into memory. For a messaging SDK this is fine in practice, but a `max_size` guard or a note in the docstring about file size limits would be defensive. Non-blocking. 2. **Truncated branch name**: Branch is `7-add-direct-message-and-file-upload-metho` (truncated). Likely Forgejo auto-generation. Cosmetic only. 3. **`__init__.py` exports**: The new `DirectMessagesMixin` and `FilesMixin` are not exported in `__init__.py`. The existing mixins (`GroupsMixin`, `MembersMixin`, `MessagesMixin`) are also not exported -- only `GroupMeClient` and exceptions are. This is consistent. Consumers access methods through `GroupMeClient`, not mixins directly. No change needed. 4. **Version bump**: `pyproject.toml` is still at `0.2.0`. If this SDK is published to Forgejo PyPI (like `pal-e-docs-sdk`), a version bump to `0.3.0` would be needed before publish. This may be handled in a separate chore PR (matching the pattern of PR #6 "chore: bump version to 0.2.0"). Non-blocking for this PR. ### SOP COMPLIANCE - [x] Branch named after issue (`7-add-direct-message-and-file-upload-metho` maps to issue #7) - [x] PR body has: Summary, Changes, Test Plan, Review Checklist, Related - [x] Related section references parent issue (`Closes #7`) and downstream dependency (`groupme-mcp#7`) - [x] Board reference present (`board-westside-basketball`) - [x] No secrets committed (token is `fake-token` in tests via conftest helper) - [x] No unnecessary file changes (5 files, all directly related to the feature) - [x] Tests exist with comprehensive coverage (23 new tests, happy path + error + edge cases) ### PROCESS OBSERVATIONS - Clean additive PR -- 523 additions, 1 deletion (the client class signature change). Zero risk of regression to existing functionality. - Deployment frequency: this unblocks `groupme-mcp#7` downstream. Good dependency chain documentation in the PR body. - The version bump for publish should be tracked as a follow-up chore issue to avoid scope creep here. - Test-to-code ratio is healthy: 359 lines of tests for 161 lines of implementation. ### VERDICT: APPROVED