Skip to content
This repository has been archived by the owner on Sep 30, 2022. It is now read-only.

Add /rban command allowing remote bans #13

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add /rban command allowing remote bans #13

wants to merge 3 commits into from

Conversation

SphericalKat
Copy link
Contributor

Sudo users can remotely ban members from chats containing the bot

Signed-off-by: ATechnoHazard [email protected]

@skittles9823
Copy link
Contributor

@ATechnoHazard Naice

@SanchithHegde
Copy link

Cool!

@ghost
Copy link

ghost commented Mar 31, 2018

@ATechnoHazard Oboi

@ghost
Copy link

ghost commented Mar 31, 2018

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]>
def rban(bot: Bot, update: Update, args: List[str]):
message = update.effective_message

if args:
Copy link
Owner

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

Copy link
Owner

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)

return ""

try:
member = chat.get_member(user_id)
Copy link
Owner

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.

if excp.message == "Reply message not found":
# Do not reply
message.reply_text('Banned!', quote=False)
return log
Copy link
Owner

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
Copy link
Owner

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":
Copy link
Owner

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.

@SphericalKat
Copy link
Contributor Author

Added the requested changes :)
Not sure if the exceptions are enough, but hoping they are :P

@ghost
Copy link

ghost commented Apr 15, 2018

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]>
@Yash-Garg
Copy link

@Yasir-siddiqui Wow

return

try:
chat = bot.get_chat(chat_id)
Copy link
Owner

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

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:
Copy link
Owner

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh crap.

Copy link
Owner

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

@PaulSonOfLars
Copy link
Owner

Are you still working on this?

@SphericalKat
Copy link
Contributor Author

SphericalKat commented Apr 28, 2018 via email

* 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]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants