Skip to content

Allow negative chat ID for Telegram#7688

Closed
olafz wants to merge 2 commits into
home-assistant:devfrom
olafz:telegram_groups
Closed

Allow negative chat ID for Telegram#7688
olafz wants to merge 2 commits into
home-assistant:devfrom
olafz:telegram_groups

Conversation

@olafz
Copy link
Copy Markdown
Contributor

@olafz olafz commented May 21, 2017

Description:

The latest version of HASS requires a positive integer for CHAT_ID in Telegram:

Invalid config for [notify.telegram]: value must be at least 0 for dictionary
value @ data['chat_id']. Got '-12345678'. (See ?, line ?). Please check the
docs at https://home-assistant.io/components/notify.telegram/

However, this breaks group chats within Telegram: if the chat is a group, the chat id is negative. If it is a single person, the chat id is positive. This PR allows for group chats again.

@mention-bot
Copy link
Copy Markdown

@olafz, thanks for your PR! By analyzing the history of the files in this pull request, we identified @azogue, @sander76 and @fabaff to be potential reviewers.

@homeassistant
Copy link
Copy Markdown
Contributor

Hi @olafz,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

@olafz olafz changed the title Fix negative chat ID for Telegram Allow negative chat ID for Telegram May 21, 2017
Copy link
Copy Markdown
Member

@azogue azogue left a comment

Choose a reason for hiding this comment

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

You missed another in line 87:

BASE_SERVICE_SCHEMA = vol.Schema({
    vol.Optional(ATTR_TARGET): vol.All(cv.ensure_list, [cv.positive_int]),

I didn't know about the negative chat_ids for groups...

Apart from that, you will have to make some changes to allow communication with groups:

  • When getting the message data (in the _get_message_data method), where the user_id is added, the chat_id should also be added, in order to be able to respond to the group instead of the user. And change the condition to something like ('text' not in msg_data and 'data' not in msg_data and 'chat' not in msg_data).
  • In process_message, when receiving a new_chat_participant message, there will be an unhandled KeyError when trying to get the non-existent 'text' field.

@olafz
Copy link
Copy Markdown
Contributor Author

olafz commented May 21, 2017

Superseded by #7689 apparently?

@olafz olafz closed this May 21, 2017
@olafz olafz deleted the telegram_groups branch May 21, 2017 10:58
@home-assistant home-assistant locked and limited conversation to collaborators Sep 4, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants