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_u… #1469
base: dev-3.x
Are you sure you want to change the base?
Conversation
✔️ Changelog found.Thank you for adding a description of the changes |
Instead #1454 |
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.
Changes should be covered by tests
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev-3.x #1469 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 422 422
Lines 10937 10938 +1
=========================================
+ Hits 10937 10938 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
@Robotvasya, do you need help writing tests or can you handle it yourself? |
I will try. And then ask for help) |
@Olegt0rr, I made some tests, please review it |
# Conflicts: # tests/test_api/test_methods/test_get_url.py
["supergroup", -100123456789, None, False, False, None, "https://t.me/c/123456789/10"], | ||
["supergroup", -100123456789, None, False, False, 3, "https://t.me/c/123456789/10"], | ||
["supergroup", -100123456789, None, False, True, None, "https://t.me/c/123456789/10"], | ||
["supergroup", -100123456789, None, False, True, 3, "https://t.me/c/123456789/3/10"], | ||
["supergroup", -100123456789, None, True, False, None, "https://t.me/c/123456789/10"], | ||
["supergroup", -100123456789, None, True, False, 3, "https://t.me/c/123456789/10"], | ||
["supergroup", -100123456789, None, True, True, None, "https://t.me/c/123456789/10"], | ||
["supergroup", -100123456789, None, True, True, 3, "https://t.me/c/123456789/3/10"], | ||
["supergroup", -100123456789, "name", False, False, None, "https://t.me/name/10"], | ||
["supergroup", -100123456789, "name", False, False, 3, "https://t.me/name/10"], | ||
["supergroup", -100123456789, "name", False, True, None, "https://t.me/name/10"], | ||
["supergroup", -100123456789, "name", False, True, 3, "https://t.me/name/3/10"], | ||
["supergroup", -10012345678, "name", True, False, None, "https://t.me/c/12345678/10"], | ||
["supergroup", -100123456789, "name", True, False, 3, "https://t.me/c/123456789/10"], | ||
["supergroup", -10012345678, "name", True, True, None, "https://t.me/c/12345678/10"], | ||
["supergroup", -100123456789, "name", True, True, 3, "https://t.me/c/123456789/3/10"], |
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.
For what purpose chat_type
and chat_id
are parametrized here?
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.
This creates different types of updates.
I planned that a “channel” type could be added here in the future. And just add a few parameters, do not completely rewrite the test. Although, as I understand it, the channels do not have topics.
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.
If this is redundant at the moment, then of course I will remove it inside the the function's body.
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.
The refactoring done.
First group of tests covers all chat_types cases.
Second covers if the supergroup chat is a forum.
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: