Skip to content

Fix telegram chats#7689

Merged
balloob merged 6 commits into
home-assistant:devfrom
azogue:fix-telegram-chats
May 22, 2017
Merged

Fix telegram chats#7689
balloob merged 6 commits into
home-assistant:devfrom
azogue:fix-telegram-chats

Conversation

@azogue
Copy link
Copy Markdown
Member

@azogue azogue commented May 21, 2017

Description:

Related issue (if applicable): fixes #7694

Fix negative chat_ids used in Telegram groups:

  • Negative chat_ids for groups.
  • Include chat_id in event data.
  • Handle KeyError when receiving other types of messages, as new_chat_member ones, and send them as text.

I know this is like a duplicate of #7688, but this has more changes to allow receiving messages from a group, not only sending them.

It's tested and intended to be included in 0.45.1

azogue added 2 commits May 21, 2017 11:58
- Negative `chat_id`s for groups.
- Include `chat_id` in event data.
- Handle KeyError when receiving other types of messages, as `new_chat_member` ones, and send them as text.
@mention-bot
Copy link
Copy Markdown

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

Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Just a minor comment.

else:
event_data[ATTR_TEXT] = data[ATTR_TEXT]
# Some other thing...
_LOGGER.warning('SOME OTHER THING RECEIVED --> "%s"', data)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Clarify to something like:

_LOGGER.warning("Message without text data received: %s", data)

Note the use of quotes. That's home assistant standard now.

@MartinHjelmare MartinHjelmare added this to the 0.45.1 milestone May 21, 2017
- fix if condition for msg bad fields.
- return True for a correct but not allowed or not recognized message: if not, the message arrives continuously.
- Allow to receive messages from unauthorized users if they come from authorized groups.
- They come as normal messages, except for the 'edited_message' field instead of 'message'.
@arsaboo
Copy link
Copy Markdown
Contributor

arsaboo commented May 21, 2017

Is this related #7696 ?

@azogue
Copy link
Copy Markdown
Member Author

azogue commented May 21, 2017

@arsaboo, not exactly, #7696 is because the changes of 0.45 imply that we now have to define a telegram_bot. This was not tagged as a breaking change ...

@sander76
Copy link
Copy Markdown
Contributor

@azogue @MartinHjelmare Somehow the telegram_bot entry entered the notify platform in the docs. This is a mistake as they are unrelated.

@MartinHjelmare
Copy link
Copy Markdown
Member

@sander76 after the refactor of #7454, telegram notify requires telegram_bot to be setup. So they are no longer unrelated.

@balloob balloob merged commit 922303f into home-assistant:dev May 22, 2017
balloob pushed a commit that referenced this pull request May 22, 2017
* bugfix for Telegram chat_ids

- Negative `chat_id`s for groups.
- Include `chat_id` in event data.
- Handle KeyError when receiving other types of messages, as `new_chat_member` ones, and send them as text.

* unused import

* fix double quote style, fix boolean expr, change warning msg

* mistake

* some more fixes

- fix if condition for msg bad fields.
- return True for a correct but not allowed or not recognized message: if not, the message arrives continuously.
- Allow to receive messages from unauthorized users if they come from authorized groups.

* support for `edited_message`s

- They come as normal messages, except for the 'edited_message' field instead of 'message'.
@balloob balloob mentioned this pull request May 22, 2017
@balloob balloob mentioned this pull request Jun 2, 2017
@azogue azogue deleted the fix-telegram-chats branch June 4, 2017 12:02
@michaelarz
Copy link
Copy Markdown

Hi! it seems to be broken again. negative chat_id calls this:
2017-06-19 18:23:56 ERROR (MainThread) [homeassistant.components.telegram_bot] Incoming message is not allowed ({'from': {'last_name': 'Last', 'first_name': 'First', 'id': 2823xxxx, 'language_code': 'en'}, 'message_id': 1963, 'text': '/ping', 'entities': [{'offset': 0, 'type': 'bot_command', 'length': 5}], 'chat': {'title': 'Title', 'all_members_are_administrators': True, 'id': -17616yyyy, 'type': 'group'}, 'date': 1497889436})
2017-06-19 18:24:04 WARNING (Thread-6) [homeassistant.components.telegram_bot] Unallowed targets: [-17616yyyy], using default: 2823xxxx

@azogue
Copy link
Copy Markdown
Member Author

azogue commented Jun 19, 2017

Hi @michaelarz, have you included the -17616yyy chat_id to the list of allowed chat ids in the telegram_bot config? Could you show us your yaml config, please?

@michaelarz
Copy link
Copy Markdown

Hi @azogue, thank you, that was exatly what I forgot to do. Sorry, my fault.

@home-assistant home-assistant locked and limited conversation to collaborators Jun 20, 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.

notify.telegram chat IDs can no longer be negative

9 participants