-
-
Notifications
You must be signed in to change notification settings - Fork 833
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added message_thread_id to url of chats with topics for message.get_url()
(#1451)
#1454
Conversation
✔️ Changelog found.Thank you for adding a description of the changes |
aiogram/types/message.py
Outdated
message_id_value = ( | ||
f"{self.message_id}" | ||
if not self.message_thread_id | ||
else f"{self.message_thread_id}/{self.message_id}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if I don't wanna get in-thread link?
You should save ability to get direct link within merged chat (also to save compatibility).
For example, both links are valid:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I tested patch manually, I tested the following cases:
-
Regular chat without topics. The link comes without a topic in the form:
https://t.me/test_chat*/53
-
Chat with topics. The link comes in the form:
https://t.me/test_chat*/2/54
.
But at the same time, posts in the default topic (General) come without a thread ID.https://t.me/test_chat*/55
-
Chat with topics, but one of the users has disabled the topic view:
- A user whose topics are disabled always writes to the main group. and the link to his post comes without a topic
https://t.me/test_chat*/55
. - If such a user reply to a message from a topic (it is marked with a topic tag) then the link to his response will contain the topic ID.
https://t.me/test_chat*/2/54
. But when this user clicks on this link, the message is found without problems in chat viewed as messages. - The link
https://t.me/test_chat*/2/54
is valid for both users. And for the one whose chat is viewed as messages and for the one who has a chat with topics.
This behavior seems like in official Telegram client.
Perhaps I haven't checked something or am missing some important idea
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But I find another bug. Let's this PR still blocking.
So if you reply to another massage, self.message_thread_id
suddenly equals the message ID of the replied message.
I back to original image of aiogram lib and made this handler:
@router.message()
async def echo_handler(message: Message) -> None:
url = message.get_url()
if url:
await message.answer(url)
await message.answer(f"thread_id = {message.message_thread_id}")
else:
await message.answer("Ссылка для частных групп и каналов недоступна")
await message.answer(f"thread_id = {message.message_thread_id}")
return
Me:
This message is #20
Bot:
thread_id = None
Me:
Reply to message #22
Bot:
thread_id = 22
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But I find another bug.
It's not a bug. Answer inherits thread_id
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps I haven't checked something or am missing some important idea
You miss case with message posted to the thread, but displayed within merged group. Look at my example - different views for the same message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked those links again.
https://t.me/aiogram/48111/67389
https://t.me/aiogram/67389
Both in the mobile client and in the web version, when you click on both of these links, the behavior is the same:
I get to the topic and see the message. And the chat looks like with topics. No differences are visible. Even if in the display settings topics are disabled and chat as messages is disabled.
But. The different behavior if I copy a link to a message. If convert chat back, so links like https://t.me/aiogram/48111/67389 will become broken.
Now I understand that this is a subtle hint at an additional function argument. I'll do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not a bug. Answer inherits thread_id
Please clarify this moment.
In general topic thread_id is None. But when we reply in main thread we receive thread_id as message_id. It looks like not inheritance.
It makes my PR useless. Because in the Telegram update we get a thread_id, which in fact is not a chat topic id, but a thread for replying to messages. It would be stupid to override the behavior of the telegram api.
Actually the problem I was trying to solve during moderation:
if a user deletes a message, then in a chat with topics, the link to the deleted message leads not to the topic but to the general. And the moderator cannot find in which topic the complaint was.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#ac987ac I have implemented the following logic:
Added the include_thread_id
parameter which is False by default.
A thread ID is added to the link only if and at the same time
- this parameter
include_thread_id
is true - the message is not in the main topic
- the message is not a reply to the another message.
message.get_url()
(#1451)message.get_url()
(#1451)
LGTM But still need to pass all checks |
#1469 new PR |
Description
Added message_thread_id to url of chats with topics for
message.get_url()
method.Fixes #1451
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Test Configuration:
Checklist: