Skip to content
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

Closed
wants to merge 0 commits into from

Conversation

Robotvasya
Copy link
Contributor

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.

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • pytest
  • black
  • manual

Test Configuration:

  • Operating System: Windows
  • Python version: Python 3.10

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@github-actions github-actions bot added the 3.x Issue or PR for stable 3.x version label Apr 8, 2024
Copy link

github-actions bot commented Apr 8, 2024

✔️ Changelog found.

Thank you for adding a description of the changes

message_id_value = (
f"{self.message_id}"
if not self.message_thread_id
else f"{self.message_thread_id}/{self.message_id}"
Copy link
Contributor

@Olegt0rr Olegt0rr Apr 8, 2024

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:

Copy link
Contributor Author

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:

  1. Regular chat without topics. The link comes without a topic in the form: https://t.me/test_chat*/53

  2. 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

  3. 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

Copy link
Contributor Author

@Robotvasya Robotvasya Apr 9, 2024

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:
https://t.me/c/2052226818/20

Bot:
thread_id = None

Me:
Reply to message #22

Bot:
https://t.me/c/2052226818/23

Bot:
thread_id = 22

Copy link
Contributor

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

Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor Author

@Robotvasya Robotvasya Apr 9, 2024

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.

Copy link
Contributor Author

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.

@Robotvasya Robotvasya changed the title Fixed links to chat with topics for message.get_url() (#1451) Added message_thread_id to url of chats with topics for message.get_url() (#1451) Apr 11, 2024
@JrooTJunior JrooTJunior self-requested a review April 12, 2024 12:30
@Olegt0rr
Copy link
Contributor

Olegt0rr commented Apr 22, 2024

LGTM

But still need to pass all checks

@Robotvasya
Copy link
Contributor Author

#1469 new PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.x Issue or PR for stable 3.x version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

message.get_url() in chat with topics -> wrong url
2 participants