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

Resolves #28: Add change emojis command #30

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions picobot/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ def main():
dp.add_handler(CommandHandler('setdefaultpack', handlers.set_default_pack))
dp.add_handler(CommandHandler('setpublic', handlers.handler_pack_public))
dp.add_handler(CommandHandler('setprivate', handlers.handler_pack_private))
dp.add_handler(CommandHandler('change_emojis',handlers.change_emojis))
media_filter = (Filters.photo | Filters.document) & (~Filters.reply)
dp.add_handler(MessageHandler(filters=media_filter, callback=handlers.caption_handler))

Expand Down
49 changes: 49 additions & 0 deletions picobot/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,55 @@ def del_sticker(bot: Bot, update: Update):
msg.reply_text(responses.REMOVE_STICKER_HELP)


def change_emojis(bot: Bot, update: Update):
msg: Message = update.message
msg_type = get_msg_type(msg)
user_id = msg.from_user.id
response = responses.ERROR_MSG
splittext = shlex.split(msg.text)

# remove sticker from pack
try:
if msg_type == MsgType.REP_STICKER:
pack_name = msg.reply_to_message.sticker.set_name
sticker_id = msg.reply_to_message.sticker.file_id

if pack_name is None:
msg.reply_text('Não é possível remover o sticker de um pack inexistente.')
return
Comment on lines +323 to +329
Copy link
Member

Choose a reason for hiding this comment

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

Faltou dizer o que acontece quando a mensagem não é um reply para um sticker. Nesse caso, daria para tratar isso antes de tudo:

Suggested change
if msg_type == MsgType.REP_STICKER:
pack_name = msg.reply_to_message.sticker.set_name
sticker_id = msg.reply_to_message.sticker.file_id
if pack_name is None:
msg.reply_text('Não é possível remover o sticker de um pack inexistente.')
return
if msg_type != MsgType.REP_STICKER:
msg.reply_text(responses.CHANGE_EMOJIS_HELP)
return
pack_name = msg.reply_to_message.sticker.set_name
sticker_id = msg.reply_to_message.sticker.file_id
if pack_name is None:
msg.reply_text(responses.PACK_DOES_NOT_EXIST)
return

Outra coisa é usar o responses para as mensagens de resposta do bot. Coloquei já na sugestão como seria o uso delas, só faltaria adicionar o CHANGE_EMOJIS_HELP e o PACK_DOES_NOT_EXIST no responses.py.


if not repository().check_permission(user_id, pack_name):
msg.reply_text(responses.NO_PERMISSION)
return

bot.delete_sticker_from_set(sticker_id)
# add_sticker(bot, update)
msg = update.message
Copy link
Contributor

Choose a reason for hiding this comment

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

Isso já é feito na primeira linha deste método.


except Exception as exc:
msg.reply_text("Changing Emojis: Falhou")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
msg.reply_text("Changing Emojis: Falhou")
msg.reply_text(responses.CHANGE_EMOJIS_HELP)

logger.error(
"Exception changing emojis for sticker in pack. User id %d Pack %s", user_id, pack_name,
)
logger.error(exc)

# get emojis
if len(splittext) > 1:
emoji = splittext[1]
else:
emoji = DEFAULT_EMOJI
Copy link
Contributor

Choose a reason for hiding this comment

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

Eu acho que não faz muito sentido ter um emoji default para este comando. Se o usuário não informar nenhum emoji seria melhor apenas retornar uma mensagem de erro.
O "autocomplete" do Telegram pra comandos de bot faz com que o comando seja enviado imediatamente, sem o usuário poder completar a mensagem com os "argumentos", então o emoji default iria mais atrapalhar do que ajudar neste caso.


if msg_type not in [MsgType.STICKER, MsgType.REP_STICKER]:
update.message.reply_text("Message is not a reply to a sticker.")
return
Comment on lines +352 to +354
Copy link
Member

@JPTIZ JPTIZ Nov 2, 2021

Choose a reason for hiding this comment

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

Aí, com as mudanças de cima isso já seria feito no início., então o if aqui seria desnecessário. Acho que daria para deixar essa verificação antes do try ainda.

Copy link
Contributor

Choose a reason for hiding this comment

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

Concordo.
Outro detalhe, por consistência as mensagens de resposta pros usuários são sempre em português (ou deveriam ser, pelo menos hehe).


if insert_sticker_in_pack(bot, msg, user_id, pack_name, emoji):
return

# check for errors
update.message.reply_text(response)


def set_default_pack(bot: Bot, update: Update):
msg: Message = update.message
user_id = msg.from_user.id
Expand Down