Add support for Telegram message attachments#153216
Add support for Telegram message attachments#153216zweckj merged 18 commits intohome-assistant:devfrom
Conversation
|
Hey there @hanwg, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
|
@hanwg I started a discussion so feel free to discuss there or here. would love your feedback https://github.com/orgs/home-assistant/discussions/1169 |
|
I'm just trying to visualize how the full flow looks like.
|
|
Thanks for the quick feedback! I suggest we split this into two parts: Second, add a service called
For future improvements, it might help to also include file type info (photo, document, audio, etc.) in the event for smarter automations. What do you think? |
|
You can include the original filename and other metadata (e.g. file type info) in the event. I haven't come across any integrations that perform downloads other than Downloader. |
I added the mime type and file name when relevant. Small question first - you think I should add the new service to this PR as well? IMO it belongs to a separate PR which I'm more than willing to implement, but I don't mind adding this here in case you think it's the same context |
|
I think we can keep them separate. |
|
@hanwg |
erwindouna
left a comment
There was a problem hiding this comment.
Looks good to me, thanks @aviadlevy! :)
|
@hanwg any chance you can take a look. I'll continue with the second PR once this PR approach will be approved and merged |
|
Overall PR looks fine but might need to document down the limitations: |
Can you provide reference to those limitations? And did you want to add it in the web documentation or as comments in the code? |
hanwg
left a comment
There was a problem hiding this comment.
The tests look identical.
Perhaps they can be parameterized using @pytest.mark.parametrize?
I actually wondered myself what's better in terms of readability, so I'll try to make that happen |
hanwg
left a comment
There was a problem hiding this comment.
Looks good to me.
I tested this and it works fine.
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for handling Telegram message attachments by introducing a new telegram_attachment event that fires when files are sent to the bot. The implementation extracts file metadata (file_id, mime_type, size, etc.) from attachment messages and makes it available for automation triggers.
Key Changes:
- Adds new
telegram_attachmentevent type with file metadata extraction - Implements support for photos and documents with appropriate metadata handling
- Adds comprehensive test coverage for attachment handling
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
homeassistant/components/telegram_bot/const.py |
Adds event constant and file attribute constants for attachment metadata |
homeassistant/components/telegram_bot/bot.py |
Implements attachment detection logic and file metadata extraction |
tests/components/telegram_bot/test_telegram_bot.py |
Adds parameterized test for photo and document attachment events |
tests/components/telegram_bot/fixtures/update_message_attachment_photo.json |
Adds test fixture with sample photo attachment data |
tests/components/telegram_bot/fixtures/update_message_attachment_document.json |
Adds test fixture with sample document attachment data |
| photos = cast(Sequence[PhotoSize], message.effective_attachment) | ||
| return { | ||
| ATTR_FILE_ID: photos[-1].file_id, | ||
| ATTR_FILE_MIME_TYPE: "image/jpeg", # telegram always uses jpeg for photos |
There was a problem hiding this comment.
The comment states 'telegram always uses jpeg for photos' but this is incorrect. Telegram can deliver photos in WebP format or JPEG depending on the client and API version. Consider using a more accurate comment like 'Telegram typically delivers photos as JPEG' or remove the hardcoded mime type and use getattr with a fallback.
| ATTR_FILE_MIME_TYPE: "image/jpeg", # telegram always uses jpeg for photos | |
| ATTR_FILE_MIME_TYPE: getattr(photos[-1], "mime_type", "image/jpeg"), # Telegram typically delivers photos as JPEG, but other formats are possible |
There was a problem hiding this comment.
PhotoSize does not hold mime_type fields. other formats other than JPEG is only available as Document.
https://stackoverflow.com/a/64927595
| return { | ||
| k: getattr(message.effective_attachment, v) | ||
| for k, v in ( | ||
| (ATTR_FILE_ID, "file_id"), | ||
| (ATTR_FILE_NAME, "file_name"), | ||
| (ATTR_FILE_MIME_TYPE, "mime_type"), | ||
| (ATTR_FILE_SIZE, "file_size"), | ||
| ) | ||
| if hasattr(message.effective_attachment, v) | ||
| } |
There was a problem hiding this comment.
[nitpick] The dictionary comprehension silently skips attributes that don't exist using hasattr. This could mask issues if expected attributes are missing. Consider logging when attributes are missing or documenting which attachment types have which attributes to make the behavior more transparent.
| return { | |
| k: getattr(message.effective_attachment, v) | |
| for k, v in ( | |
| (ATTR_FILE_ID, "file_id"), | |
| (ATTR_FILE_NAME, "file_name"), | |
| (ATTR_FILE_MIME_TYPE, "mime_type"), | |
| (ATTR_FILE_SIZE, "file_size"), | |
| ) | |
| if hasattr(message.effective_attachment, v) | |
| } | |
| result = {} | |
| for k, v in ( | |
| (ATTR_FILE_ID, "file_id"), | |
| (ATTR_FILE_NAME, "file_name"), | |
| (ATTR_FILE_MIME_TYPE, "mime_type"), | |
| (ATTR_FILE_SIZE, "file_size"), | |
| ): | |
| if hasattr(message.effective_attachment, v): | |
| result[k] = getattr(message.effective_attachment, v) | |
| else: | |
| _LOGGER.debug( | |
| "Attachment of type %s is missing expected attribute '%s'", | |
| type(message.effective_attachment).__name__, | |
| v, | |
| ) | |
| return result |
There was a problem hiding this comment.
I really don't think it's necessary to have that log, but willing to change that if anyone think otherwise
This comment was marked as off-topic.
This comment was marked as off-topic.
|
Please do not ping reviewers though, thanks! |
Breaking change
Nope
Proposed change
This PR adds support for Telegram message attachments by firing a new event with the attachment’s file_id, enabling automations to handle files sent to the bot.
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: