-
Notifications
You must be signed in to change notification settings - Fork 918
Add /rban command allowing remote bans #13
base: master
Are you sure you want to change the base?
Conversation
@ATechnoHazard Naice |
Cool! |
@ATechnoHazard Oboi |
Pro |
Sudo users can remotely ban members from chats containing the bot Format for use: /rban <@username/userID> <chatID> Signed-off-by: ATechnoHazard <[email protected]>
tg_bot/modules/bans.py
Outdated
def rban(bot: Bot, update: Update, args: List[str]): | ||
message = update.effective_message | ||
|
||
if args: |
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.
Use if not args
and return immediately
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.
(makes it easier to read + avoids unnecessary indents)
tg_bot/modules/bans.py
Outdated
return "" | ||
|
||
try: | ||
member = chat.get_member(user_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.
Check if chat is a group chat first. If you've loaded a private chat, how are you gonna ban?
Also, check if bot is admin in that chat.
tg_bot/modules/bans.py
Outdated
if excp.message == "Reply message not found": | ||
# Do not reply | ||
message.reply_text('Banned!', quote=False) | ||
return log |
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.
log
not declared.
@@ -191,6 +191,66 @@ def unban(bot: Bot, update: Update, args: List[str]) -> str: | |||
return log | |||
|
|||
|
|||
@run_async | |||
@bot_admin |
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.
All the return ""
are useless, given this hasn't been set as @loggable. Use normal return
instead.
chat.kick_member(user_id) | ||
message.reply_text("Banned!") | ||
except BadRequest as excp: | ||
if excp.message == "Reply message not found": |
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.
You're going to need more exception catching here, see the possible gban exceptions.
Added the requested changes :) |
Cool |
1. Use `if not` and return immediately 2. Add checks for private chat and bot admin+restrict status 3. Remove useless "" and log 4. Add more exception handling. Signed-off-by: ATechnoHazard <[email protected]>
@Yasir-siddiqui Wow |
tg_bot/modules/bans.py
Outdated
return | ||
|
||
try: | ||
chat = bot.get_chat(chat_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.
Small nitpick - if people write after the chat_id, that text will be included. how about .split() and take [0]? avoid as many errors as possible, as early as possible
tg_bot/modules/bans.py
Outdated
message.reply_text("I'm sorry, but that's a private chat!") | ||
return | ||
|
||
if not is_bot_admin(chat, bot.id) and not chat.get_member(bot.id).can_restrict_members: |
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.
you mean or
don't you?
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.
Oh crap.
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.
De Morgan law bugs - this is why code review is good
Are you still working on this? |
Yeah lol, just have exams rn :P
…On Sat 28 Apr, 2018, 7:15 PM Paul Larsen, ***@***.***> wrote:
Are you still working on this?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#13 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AeSls9vK6kUoTmNAmKyAn8cWyXAjpE9-ks5ttHJ5gaJpZM4TCcgb>
.
|
* Fix de-morgan logic * Add `split()` to chat_id to ignore text after the chat id * Refactor exception handling, like in global_bans module * Add a missing import Signed-off-by: ATechnoHazard <[email protected]>
Sudo users can remotely ban members from chats containing the bot
Signed-off-by: ATechnoHazard [email protected]